Decide how to test numba functions

[eric-czech]

Testing and getting coverage on jit-compiled functions presents a couple challenges. In https://github.com/pystatgen/sgkit/pull/76, I started by creating separate variables for jit-functions and testing them alongside the original function as part of the same test run. Two reasons for this are that:

1. There are tests that should only be run using the jit-compiled function since they would be prohibitively slow otherwise
2. Code coverage is only possible on the original functions

One alternative to this would be to not have separate versions of each function and do this instead:

• Run all unit tests with coverage and NUMBA_DISABLE_JIT turned on, and read this env variable in the tests to skip some that would otherwise be prohibitively slow
• Disable coverage, disable NUMBA_DISABLE_JIT (so jit is on), and run a subset of the tests again that actually reference JIT-compiled functions

I think this alternative is better, but I'm not sure what a good way is to limit the scope of tests run with JIT on.

[ravwojdyla]

Just wanted to mention pytest markers, it doesn't solve the main issue but might be useful here.

[daletovar]

Why is code coverage only possible on the original functions? I like your alternative if that's the case. I don't think it's important for #76, but I think in most cases we should have nogil=True when using njit.

[eric-czech]

Why is code coverage only possible on the original functions?

Hey @daletovar! I'm not sure except that codecov recognizes that there's code to be covered in an @njit function but then goes on to fail to recognize any tests that force all the code paths within it. Perhaps it decides what should be covered based on the source code and then decides what is covered based on parsed/compiled code. That sounds like a reasonable way for it to work but I really have no idea what it does internally.

[daletovar]

@eric-czech, thanks for clarifying all of that for me. I'm still +1 on your alternative idea.

[tomwhite]

I wonder if we can close this given that we are turning off coverage for numba functions (see #380 and linked issue)? I think reviewers just need to be careful to check that all new numba functions have unit tests.

[tomwhite]

Fixed in #964