duetosymmetry / qnm

Python package for computing Kerr quasinormal mode frequencies, separation constants, and spherical-spheroidal mixing coefficients
https://qnm.readthedocs.io
MIT License
39 stars 19 forks source link

qnm incompatible with numpy 2 #31

Open duncanmmacleod opened 1 month ago

duncanmmacleod commented 1 month ago

This project is incompatible with numpy 2, meaning new deployments fail pretty quickly:

Consider this snippet:

python3 -m venv test
./test/bin/python3 -m pip install qnm
./test/bin/python3 -c "import qnm"

which installs this set of packages:

Successfully installed llvmlite-0.43.0 numba-0.60.0 numpy-2.0.2 qnm-0.4.3 scipy-1.14.1 tqdm-4.66.5 

which results in this error:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/duncan/tmp/test/lib/python3.11/site-packages/qnm/__init__.py", line 72, in <module>
    from . import radial
  File "/home/duncan/tmp/test/lib/python3.11/site-packages/qnm/radial.py", line 9, in <module>
    from .contfrac import lentz
  File "/home/duncan/tmp/test/lib/python3.11/site-packages/qnm/contfrac.py", line 12, in <module>
    def lentz(a, b, tol=1.e-10, N_min=0, N_max=np.Inf, tiny=1.e-30):
                                               ^^^^^^
  File "/home/duncan/tmp/test/lib/python3.11/site-packages/numpy/__init__.py", line 397, in __getattr__
    raise AttributeError(
AttributeError: `np.Inf` was removed in the NumPy 2.0 release. Use `np.inf` instead.. Did you mean: 'inf'?

It looks like a fairly trivial change for this error, but this might be the tip of the proverbial iceberg.

duetosymmetry commented 1 month ago

Thanks for finding this and helping to improve the package, @duncanmmacleod! I have been meaning to update the package for a long time now but didn't get a chance... One of the big changes I want to make is doing away with using pickling to store data, since it makes the code more brittle (a user might end up trying to load a pickle from a different version of the code, which would lead to bugs that can't be explained by looking at just one version of the code). I meant to make that change at the same time as updating to more modern python and numpy, but maybe I shouldn't let perfect be the enemy of the good...

Background out of the way — do you recommend some automatic way to test the full python/numpy/scipy/etc. matrix of versions? Or is this overkill, and I should just pick minimum versions of each one to try to support?

duncanmmacleod commented 1 month ago

Background out of the way — do you recommend some automatic way to test the full python/numpy/scipy/etc. matrix of versions? Or is this overkill, and I should just pick minimum versions of each one to try to support?

Github actions has good support for job matrices, but I would recommend only using that to test the various Python versions. It looks like you have that configured already, but the 'matrix' only includes a single chosen version to test:

https://github.com/duetosymmetry/qnm/blob/9008d69b22d7bb708886cf8f5ffeef5fb5a29bf4/.github/workflows/pytest-slow.yml#L13

As for numpy/scipy what I would recommend is updating the package metadata (setup.py, but you could migrate to pyproject.toml) to include a minimum specification for each of those packages, and then configuring a github actions workflow to install those minimum versions and test those (this example from the GWpy project (before it was migrated to gitlab.com) does that, but it's a bit brute force). That way you test the oldest supported version, and the newest version (which will be the default in the 'normal' test job) - testing everything in between is indeed overkill.

I might also recommend that the 'normal' test job matrix (probably the 'not slow' pytest workflow) be configured to run nightly, so you can catch incompatibilities as soon as they are released upstream. That may any up being annoying for your developer workflow, but it's one way to discover things automatically (the relevant cron syntax is included in the GWpy workflow for testing minimal dependencies, if you're interested).