Migrate Python tests to pytest

[lgarrison]

From #258, we had an OpenMP error that only showed up in the Python tests. Therefore we decided to be a bit more organized about the Python tests, and migrate to pytest. The code is pretty compact, and tests all ISA for all main API functions.

These tests all use the same data and configuration as the C tests, and hence we have the external, correct results to compare against.

Once this gets into master, we can merge/rebase master back into linearbinning. I have a corresponding test_lin.py that we can put into linearbinning once that happens.

[pep8speaks]

Hello @lgarrison! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file Corrfunc/__init__.py:

Line 20:80: E501 line too long (103 > 79 characters)

• In the file Corrfunc/io.py:

Line 43:1: W293 blank line contains whitespace Line 136:50: E231 missing whitespace after ',' Line 136:54: E231 missing whitespace after ','

• In the file Corrfunc/tests/common.py:

Line 9:1: E302 expected 2 blank lines, found 1 Line 11:1: W293 blank line contains whitespace Line 13:21: E128 continuation line under-indented for visual indent Line 15:1: W293 blank line contains whitespace Line 22:17: E128 continuation line under-indented for visual indent Line 23:7: E231 missing whitespace after ',' Line 23:11: E231 missing whitespace after ',' Line 23:14: E231 missing whitespace after ',' Line 24:1: W293 blank line contains whitespace Line 31:17: E128 continuation line under-indented for visual indent Line 32:7: E231 missing whitespace after ',' Line 32:11: E231 missing whitespace after ',' Line 32:14: E231 missing whitespace after ',' Line 33:1: W293 blank line contains whitespace Line 40:1: W293 blank line contains whitespace Line 45:1: W293 blank line contains whitespace Line 50:80: E501 line too long (91 > 79 characters) Line 52:1: W293 blank line contains whitespace Line 56:22: E231 missing whitespace after ',' Line 57:1: W293 blank line contains whitespace Line 59:26: E231 missing whitespace after ',' Line 59:34: E231 missing whitespace after ',' Line 59:40: E231 missing whitespace after ',' Line 62:20: E231 missing whitespace after ',' Line 63:26: E231 missing whitespace after ',' Line 64:1: W293 blank line contains whitespace Line 76:1: W293 blank line contains whitespace Line 77:22: E231 missing whitespace after ',' Line 77:34: E231 missing whitespace after ',' Line 78:23: E231 missing whitespace after ',' Line 78:34: E231 missing whitespace after ',' Line 79:16: E711 comparison to None should be 'if cond is not None:' Line 83:1: W293 blank line contains whitespace Line 86:25: E124 closing bracket does not match visual indentation Line 87:1: W293 blank line contains whitespace Line 92:1: E302 expected 2 blank lines, found 1 Line 96:1: W293 blank line contains whitespace Line 98:51: E231 missing whitespace after ',' Line 100:25: E124 closing bracket does not match visual indentation

• In the file Corrfunc/tests/test_mocks.py:

Line 12:1: W293 blank line contains whitespace Line 17:1: W293 blank line contains whitespace Line 20:22: E127 continuation line over-indented for visual indent Line 23:1: W293 blank line contains whitespace Line 24:7: E231 missing whitespace after ',' Line 24:11: E231 missing whitespace after ',' Line 24:14: E231 missing whitespace after ',' Line 26:31: E128 continuation line under-indented for visual indent Line 27:31: E128 continuation line under-indented for visual indent Line 28:31: E128 continuation line under-indented for visual indent Line 29:31: E128 continuation line under-indented for visual indent Line 30:31: E128 continuation line under-indented for visual indent Line 31:1: W293 blank line contains whitespace Line 33:21: E128 continuation line under-indented for visual indent Line 34:80: E501 line too long (98 > 79 characters) Line 36:1: W293 blank line contains whitespace Line 40:1: W293 blank line contains whitespace Line 42:22: E127 continuation line over-indented for visual indent Line 47:1: W293 blank line contains whitespace Line 48:7: E231 missing whitespace after ',' Line 48:11: E231 missing whitespace after ',' Line 48:14: E231 missing whitespace after ',' Line 50:31: E128 continuation line under-indented for visual indent Line 51:31: E128 continuation line under-indented for visual indent Line 52:31: E128 continuation line under-indented for visual indent Line 53:31: E128 continuation line under-indented for visual indent Line 54:31: E128 continuation line under-indented for visual indent Line 55:1: W293 blank line contains whitespace Line 57:21: E128 continuation line under-indented for visual indent Line 58:80: E501 line too long (96 > 79 characters) Line 59:1: W293 blank line contains whitespace Line 60:1: W293 blank line contains whitespace Line 64:1: W293 blank line contains whitespace Line 67:22: E127 continuation line over-indented for visual indent Line 68:1: W293 blank line contains whitespace Line 69:7: E231 missing whitespace after ',' Line 69:11: E231 missing whitespace after ',' Line 69:14: E231 missing whitespace after ',' Line 71:31: E128 continuation line under-indented for visual indent Line 72:31: E128 continuation line under-indented for visual indent Line 73:31: E128 continuation line under-indented for visual indent Line 74:31: E128 continuation line under-indented for visual indent Line 75:31: E128 continuation line under-indented for visual indent Line 76:1: W293 blank line contains whitespace Line 78:21: E128 continuation line under-indented for visual indent Line 79:80: E501 line too long (102 > 79 characters) Line 85:1: W293 blank line contains whitespace Line 98:1: W293 blank line contains whitespace Line 100:22: E127 continuation line over-indented for visual indent Line 101:1: W293 blank line contains whitespace Line 102:7: E231 missing whitespace after ',' Line 102:11: E231 missing whitespace after ',' Line 102:14: E231 missing whitespace after ',' Line 104:37: E127 continuation line over-indented for visual indent Line 105:37: E127 continuation line over-indented for visual indent Line 106:37: E127 continuation line over-indented for visual indent Line 106:62: W291 trailing whitespace Line 107:37: E127 continuation line over-indented for visual indent Line 108:1: W293 blank line contains whitespace Line 110:21: E128 continuation line under-indented for visual indent Line 112:1: W293 blank line contains whitespace Line 112:1: W391 blank line at end of file

• In the file Corrfunc/tests/test_theory.py:

Line 17:1: W293 blank line contains whitespace Line 20:22: E127 continuation line over-indented for visual indent Line 23:1: W293 blank line contains whitespace Line 24:6: E231 missing whitespace after ',' Line 24:8: E231 missing whitespace after ',' Line 24:10: E231 missing whitespace after ',' Line 26:29: E127 continuation line over-indented for visual indent Line 27:29: E127 continuation line over-indented for visual indent Line 28:29: E127 continuation line over-indented for visual indent Line 29:29: E127 continuation line over-indented for visual indent Line 30:1: W293 blank line contains whitespace Line 32:21: E128 continuation line under-indented for visual indent Line 35:1: W293 blank line contains whitespace Line 40:1: W293 blank line contains whitespace Line 44:22: E127 continuation line over-indented for visual indent Line 47:1: W293 blank line contains whitespace Line 48:6: E231 missing whitespace after ',' Line 48:8: E231 missing whitespace after ',' Line 48:10: E231 missing whitespace after ',' Line 54:1: W293 blank line contains whitespace Line 56:21: E128 continuation line under-indented for visual indent Line 57:80: E501 line too long (92 > 79 characters) Line 59:1: W293 blank line contains whitespace Line 63:1: W293 blank line contains whitespace Line 66:22: E127 continuation line over-indented for visual indent Line 71:1: W293 blank line contains whitespace Line 72:6: E231 missing whitespace after ',' Line 72:8: E231 missing whitespace after ',' Line 72:10: E231 missing whitespace after ',' Line 74:29: E127 continuation line over-indented for visual indent Line 75:29: E127 continuation line over-indented for visual indent Line 76:29: E127 continuation line over-indented for visual indent Line 77:29: E127 continuation line over-indented for visual indent Line 78:29: E127 continuation line over-indented for visual indent Line 79:29: E127 continuation line over-indented for visual indent Line 80:1: W293 blank line contains whitespace Line 82:21: E128 continuation line under-indented for visual indent Line 83:80: E501 line too long (90 > 79 characters) Line 84:1: W293 blank line contains whitespace Line 85:1: W293 blank line contains whitespace Line 86:80: E501 line too long (94 > 79 characters) Line 89:1: W293 blank line contains whitespace Line 93:22: E127 continuation line over-indented for visual indent Line 94:1: W293 blank line contains whitespace Line 95:6: E231 missing whitespace after ',' Line 95:8: E231 missing whitespace after ',' Line 95:10: E231 missing whitespace after ',' Line 97:29: E127 continuation line over-indented for visual indent Line 98:29: E127 continuation line over-indented for visual indent Line 99:29: E127 continuation line over-indented for visual indent Line 100:29: E127 continuation line over-indented for visual indent Line 101:1: W293 blank line contains whitespace Line 103:21: E128 continuation line under-indented for visual indent Line 104:80: E501 line too long (82 > 79 characters) Line 106:1: W293 blank line contains whitespace Line 111:1: W293 blank line contains whitespace Line 114:22: E127 continuation line over-indented for visual indent Line 115:1: W293 blank line contains whitespace Line 116:6: E231 missing whitespace after ',' Line 116:8: E231 missing whitespace after ',' Line 116:10: E231 missing whitespace after ',' Line 118:29: E127 continuation line over-indented for visual indent Line 119:29: E127 continuation line over-indented for visual indent Line 120:29: E127 continuation line over-indented for visual indent Line 121:29: E127 continuation line over-indented for visual indent Line 122:1: W293 blank line contains whitespace Line 124:21: E128 continuation line under-indented for visual indent Line 125:80: E501 line too long (104 > 79 characters) Line 126:1: W293 blank line contains whitespace Line 131:1: W293 blank line contains whitespace Line 139:1: W293 blank line contains whitespace Line 140:6: E231 missing whitespace after ',' Line 140:8: E231 missing whitespace after ',' Line 140:10: E231 missing whitespace after ',' Line 141:16: E221 multiple spaces before operator Line 142:27: E127 continuation line over-indented for visual indent Line 143:27: E127 continuation line over-indented for visual indent Line 144:5: E265 block comment should start with '# ' Line 144:80: E501 line too long (106 > 79 characters) Line 146:21: E128 continuation line under-indented for visual indent

Comment last updated at 2021-12-02 15:50:15 UTC

[lgarrison]

@manodeep the Python 2.7 tests have trouble with importing this module structure. And looking ahead to test_lin.py, Python 2.7 doesn't like some of the syntax there. Okay if I drop Python 2.7 from the Python test stage of the CI? Or even remove it from the CI matrix completely, unless we want to keep testing the build of the C extensions against Python 2.7.

[manodeep]

Only just got a chance to take a look - not sure I follow what's going on with the test failure - could it just be a missing __init__.py file?

We should try to support python 2.7 for at least Corrfunc v2.5, and then drop the py2.7 support from Corrfunc v2.6. Would it work to add a deprecation warning in v2.5?

[manodeep]

We should also test for some values of nthreads - say [1, 2, 3, 4] - perhaps with isa=fastest. That would (hopefully) detect any OpenMP threading issues

[lgarrison]

This version tests all ISA for the max number of threads (inferred from cores/affinity), and thread counts from 1 to min(4,maxthreads) for the fastest ISA. Unfortunately, that's a lot of combos, so CI time has increased quite a bit; the Python tests alone take over 10 minutes. I'm open to suggestions on smart ways to reduce the number of tests...

In addition, this does add one test case with nthreads = ncores + 1, just to check for correctness in that corner case.

[manodeep]

Looking at the CI failure, looks like os.cpu_count() does not exist in python2.7. That AttributeError should be fixable by a try but setting the actual number of threads correctly might be more difficult. Saw this solution on SO - which we can rip off and keep only the POSIX relevant bits. What do you think?

[lgarrison]

Working now, I think!

[manodeep]

Still haven't had time to review - will look in the next 24-48 hours at the latest!

[lgarrison]

Here's a version that refactors the reference check to use column names where possible! And, most importantly, the npairs check now uses an integer equality test rather than some tolerance.

On the runtime, I'm considering the fact that the ISA sweep already tests the multi-threaded correctness. So what about reducing the thread sweep to two values: 1 and ncore+1. This tests single-threaded and "oversubscribed" correctness. Because the ISA sweep uses ncores threads, ncores+1 also ensures we test both an even and odd number of threads. I think that's pretty good coverage: single-threaded, even-number multithreaded, odd-number multithreaded, and oversubscribed multithreaded.

[manodeep]

Everything looks good to me - I made some minor comments, mostly related to whitespace and (likely) pytests wizardry

[lgarrison]

Should I go ahead and reduce the thread test cases as proposed above?

[manodeep]

Should I go ahead and reduce the thread test cases as proposed above?

Ahh yes - that should be a good enough nthreads coverage. And as long as the single thread agrees with a few different multiple-thread, we are probably okay (haven't tested that though)

[lgarrison]

Great -- single- & multi-threaded both agree with the known answer, so this is a pretty good test I think.

I'll merge this and then create a test_lin.py in linearbinning, and once that passes we'll be ready to merge linearbinning!