ntessore / healpix

Python and C package for HEALPix discretisation of the sphere
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

force numpy>2.0 instead of deprecated oldest-supported-numpy during build #55

Closed sroet closed 3 weeks ago

sroet commented 4 weeks ago

closes #54 (specifically if a release is done somewhat soon after merging this)

After toying a bit with the local installation, this change actually resolved the error.

apparently oldest-supported-numpy is deprecated

It shouldn't break installs as this is mentioned on the numpy docs nowadays:

By default, NumPy will expose an API that is backwards compatible with the oldest NumPy version that supports the currently oldest compatible Python version. NumPy 1.25.0 supports Python 3.9 and higher and NumPy 1.19 is the first version to support Python 3.9. Thus, we guarantee that, when using defaults, NumPy 1.25 will expose a C-API compatible with NumPy 1.19. (the exact version is set within NumPy-internal header files).

Which wasn't the case before numpy 1.25, and the original reason for the existence of oldest-supported-numpy

sroet commented 3 weeks ago

@ntessore This is ready for a review and testing, but you need to approve the action as I have never commited to this repo before (assuming default security settings)

ntessore commented 3 weeks ago

Hi @sroet, thanks for this PR. Can we keep Python 3.7 and 3.8 working by using oldest-supported-numpy on these versions, combined with an upper bound on numpy<2?

sroet commented 3 weeks ago

Hey @ntessore

Can we keep Python 3.7 and 3.8 working by using oldest-supported-numpy on these versions, combined with an upper bound on numpy<2?

I would strongly advice you to follow SPEC0/NEP29 and drop support for python<3.10 and numpy<1.23.

In the mean time I will see if there is a way to make your request work

sroet commented 3 weeks ago

@ntessore, you would have to re-approve the tests again

dd6c2df re-enables the same build environment as the current main branch for python <3.9

Be aware though, that it might lead to broken builds (already on the current main) as this pulls in numpy==1.24.4 during builds in python=3.8 while it should be python==1.17.3.

ntessore commented 3 weeks ago

Thanks @sroet! I have no strong feelings one way or the other about supporting Python 3.7 to 3.9, but this package is very minimalistic in its actual requirements. Are the wheels for 3.10+ in any way negatively affected by keeping the <=3.9 versions around?

sroet commented 3 weeks ago

I have no strong feelings one way or the other about supporting Python 3.7 to 3.9, but this package is very minimalistic in its actual requirements.

Understandable, just wanted to make you aware that there are standards that are followed to reduce maintenance burden if required. I agree that this isn't probably needed with the minimalistic requirements

Are the wheels for 3.10+ in any way negatively affected by keeping the <=3.9 versions around?

No, as far as I'm aware (just a slightly harder to read build-requirements list) (the split is 3.9+ vs <=3.8 in this PR)

If instead you meant about only supporting 3.10+ , then you would also change requires-python = ">=3.7" to requires-python = ">=3.10" and no future wheels will be build for the older pythons (older wheels will stay available for those python versions on pypi)

ntessore commented 3 weeks ago

Ok, as long as it's just the one hoop to jump through, it seems fair to keep support as it is. If it becomes more than one hoop, or the wheels start breaking, we can change the support matrix.

Thanks a lot for the PR, give me a thumbs up if you are happy for me to merge. I will release a new version soon after.

sroet commented 3 weeks ago

give me a thumbs up if you are happy for me to merge

👍 from me. Thanks for the swift response and for writing such a useful and lightweight wrapper for healpix!