zrnsm / pyculiarity

A Python port of Twitter's AnomalyDetection R Package
GNU General Public License v3.0
364 stars 146 forks source link

Remaining Python3 test failure, next PyPI release #25

Closed zrnsm closed 6 years ago

zrnsm commented 6 years ago

@michael-erasmus

Hey Michael (and Eric if you're still around, it looks like your account is gone)

I've merged and things are looking good under Python2, but I'm still seeing a single test failure under 3. I'd appreciate it if you guys could try to reproduce, debug.

I'm on an up to date Ubuntu box testing against the system Pythons which are 2.7.15rc1 and 3.6.5. I've nuked all the dependencies and started from scratch via setup.py under both, so I should have all the latest PyPi versions in each case. I'm running the tests under 2 with:

nosetests .

and under 3 with

nosetests3 .

The following test is failing for me in the second case:

FAIL: test_both_directions_e_value_threshold_med_max (test_vec.TestVec)
    eq_(len(results['anoms'].iloc[:,1]), 6)
AssertionError: 48 != 6

Obviously it would be nicer to have a more isolated and repeatable way to deal with the tests and dependencies across different machines. I'm open to suggestions if you guys have ideas there.

ericist commented 6 years ago

Just found the problem, my initial guess was correct that the problem is / being integer division in Python 2 and float division in Python 3 (assuming both arguments are ints).

The culprit is here: https://github.com/nicolasmiller/pyculiarity/blob/b30f58c92c8d18c918f712a88ee50bf174003c0e/pyculiarity/detect_vec.py#L210

This is also the only division where both arguments are ints, so the rest should be ok. I'll open a pull request.

zrnsm commented 6 years ago

Cool. Thanks.