[REVIEW] Use Numba 0.49+ API where required

[dicta]

numba/numba#5197 refactors many of Numba's submodules. Mirror the required import changes in cusignal.

This keeps cusignal in sync with numba master and the 0.49 release candidate but breaks compatibility with the current numba release (0.48).

This should go in eventually but also needs to make sure we keep compatibility with whatever version of numba we're getting in the conda channels selected by our packaging (and certainly updates the minimum required version) - need help with this part.

[GPUtester]

Can one of the admins verify this patch?

[awthomp]

rerun tests

[awthomp]

@BradReesWork -- can you comment on this part:

This should go in eventually but also needs to make sure we keep compatibility with whatever version of numba we're getting in the conda channels selected by our packaging (and certainly updates the minimum required version) - need help with this part.

I'm guessing we just need to be more explicit on the Numba version used in https://github.com/rapidsai/cusignal/blob/branch-0.14/conda/environments/cusignal_base.yml

[awthomp]

rerun tests

[mnicely]

@gmarkall Do you know when Numba 49 will be out of RC and be considered stable/latest?

[gmarkall]

@mnicely I think this may occur within the next couple of weeks.

[gmarkall]

Also - the original import (from numba.types.scalars import Complex) should have been working with 0.49 because there are shim modules that enable importing from the old path, but there were various issues with the shims, e.g.:

numba/numba#5528
numba/numba#5484
numba/numba#5487
numba/numba#5466

Once 0.49RC2 is out, the original code may (should!) still work.

If it's preferred to avoid being more specific about the version of Numba we use, something like the following should provide compatibility with both versions:

diff --git a/python/cusignal/_upfirdn.py b/python/cusignal/_upfirdn.py
index 033d5ac..ed5a17d 100644
--- a/python/cusignal/_upfirdn.py
+++ b/python/cusignal/_upfirdn.py
@@ -17,7 +17,12 @@ from string import Template
 
 import cupy as cp
 from numba import complex64, complex128, cuda, float32, float64, int64, void
-from numba.core.types.scalars import Complex
+try:
+    # Numba <= 0.49
+    from numba.types.scalars import Complex
+except ImportError:
+    # Numba >= 0.49
+    from numba.core.types.scalars import Complex
 
 _numba_kernel_cache = {}
 _cupy_kernel_cache = {}

[awthomp]

I like @gmarkall's suggestion on handling the discrepancies in Numba versions:

#53 (comment)

try:
    # Numba <= 0.49
    from numba.types.scalars import Complex
except ImportError:
    # Numba >= 0.49
    from numba.core.types.scalars import Complex

[boonedoggle]

If you try the import in that order with a try/except, you will get the following warning every time cusignal is imported:

Import requested from: 'numba.types', please update to use 'numba.core.types' or pin to
Numba version 0.48.0. This alias will not be present in Numba version 0.50.0.

Maybe reverse the order? This seems to suppress the warning for both cases.

try:
    # Numba >= 0.49
    from numba.core.types.scalars import Complex
except ImportError:
    # Numba <= 0.49
    from numba.types.scalars import Complex

[BradReesWork]

@BradReesWork -- can you comment on this part:

This should go in eventually but also needs to make sure we keep compatibility with whatever version of numba we're getting in the conda channels selected by our packaging (and certainly updates the minimum required version) - need help with this part.

I'm guessing we just need to be more explicit on the Numba version used in https://github.com/rapidsai/cusignal/blob/branch-0.14/conda/environments/cusignal_base.yml

You should have min / max version numbers for external libraries in the conda environment file

[awthomp]

These should be depreciated, approving to merge PR

[awthomp]

rerun tests

[awthomp]

Weird -- looks like my push to @dicta's fork isn't updating the main PR if you look at 'files changed'. Let's confirm that 36ed41b gets included in the merge.

[awthomp]

Weird -- looks like my push to @dicta's fork isn't updating the main PR if you look at 'files changed'. Let's confirm that 36ed41b gets included in the merge.

Nevermind -- they're showing now.

[awthomp]

rerun tests

[awthomp]

@raydouglass @BradReesWork Looks like CI is stuck on this PR. Any chance ya'll can look behind the curtain so we can get this merged?

[raydouglass]

ok to test