epics-base / p4p

Python bindings for the PVAccess network client and server.
BSD 3-Clause "New" or "Revised" License
27 stars 38 forks source link

Update Numpy and Cython for Python 3.10 and 3.11 builds #152

Closed OCopping closed 2 months ago

OCopping commented 3 months ago

This PR sets the wheel builds for Python 3.10 and 3.11 to use Numpy 2.0.1

mdavidsaver commented 3 months ago

With reference to the discussion starting here https://github.com/epics-base/p4p/issues/145#issuecomment-2289890558

I think some changes and additions will be necessary.

  1. In setup.py, conditional computing of install_requires to use the existing "assume ABI forward compatibility ..." computed range when building against numpy 1.x, and numpy >= 1.7 when building against 2.x.
  2. In src/p4p.h, define NPY_NO_DEPRECATED_API unconditionally, and add a definition of NPY_TARGET_VERSION to 1.7.
  3. In pyproject.toml, I think the numpy dependency would simply be numpy >= 1.7. The specific versions in .github/workflows/build.yml should take precedence for pypi.org builds, and would let users continue to build from source against numpy 1.x .

This would clearly state that numpy 1.7 is the minimum support by P4P.

eg. when building against numpy 1.10.0, the install_requires would end up as: ["numpy >= 1.10.0", "numpy < 2"].

eg. when building against numpy 2.0.0, the install_requires would end up as: ["numpy >= 1.7", "numpy < 3"].

As mentioned at the bottom at some point it would be good to test the backwards compatibility at the end of a 2.x build by rolling back numpy and re-running the unit tests. I don't see this as a blocker for this PR.

OCopping commented 3 months ago
  1. In src/p4p.h, define NPY_NO_DEPRECATED_API unconditionally, and add a definition of NPY_TARGET_VERSION to 1.7.

It seems that removing the condition for this breaks the builds... #if CYTHON_HEX_VERSION>=0x03000000

OCopping commented 3 months ago

Also NumpyVersion was only added to numpy in version 1.9, so we would either have to try/except around the call, or set the floor to 1.9 if we were to stick with the approach

mdavidsaver commented 3 months ago

It seems that removing the condition for this breaks the builds... #if CYTHON_HEX_VERSION>=0x03000000

Replacing NPY_CARRAY_RO with NPY_ARRAY_CARRAY_RO resolves this for me.

mdavidsaver commented 3 months ago

Also NumpyVersion was only added to numpy in version 1.9, ...

A negative hasattr() test for NumpyVersion seems like a solid < 2.0 indicator.

Even with these changes, I am still seeing a couple of build failures.

One appears to be a bug in numpy 2.0.1 building with MSVC. fatal error C1021: invalid preprocessor command 'warning' in numpyconfig.h. This is legit as # warning ... is a GNU extension. Interesting that their CI didn't catch this...

https://github.com/numpy/numpy/blob/282c79ebc9b61ebc5f0755736ec953b8d954922c/numpy/_core/include/numpy/numpyconfig.h#L133

The Linux py3.9 manylinux2010_x86_64 job does not find a numpy 2.0.1. Indeed there is no manylinux wheel for 3.9

ERROR: Could not find a version that satisfies the requirement numpy==2.0.1 (from versions: 1.19.3, 1.19.4, 1.19.5, 1.20.0, 1.20.1, 1.20.2, 1.20.3, 1.21.0, 1.21.1, 1.21.2, 1.21.3, 1.21.4, 1.21.5, 1.21.6)

The OSX 3.7 build fails with what I hope is a caching glitch.

Downloading ply-3.11-py2.py3-none-any.whl (49 kB)
        Expected sha256 80a41edf64a3626e729a62df7dd278474fc1726836552b67a8c6396fd7e86760
             Got        8f1d6e470697953dcaad90e3922e9b9b543e1754524552b01ad6b394b6e134e6
mdavidsaver commented 3 months ago

appears to be a bug in numpy 2.0.1 building with MSVC

https://github.com/numpy/numpy/issues/27224

OCopping commented 3 months ago

The Linux py3.9 manylinux2010_x86_64 job does not find a numpy 2.0.1.

I have a fork with a branch looking at updating supported versions and images (including arguing for the removal of Python 2.7 and 3.5 support), and using manylinux_2_28_x86_64 has Numpy 2.0 for Python 3.9

https://github.com/OCopping/p4p/pull/3

OCopping commented 3 months ago

@mdavidsaver are you happy to merge this PR in so we can make another beta release to allow people to test their builds with Python 3.10 and 3.11? We can then merge in the Numpy fix for Windows builds in another PR?

mdavidsaver commented 3 months ago

I made another iteration to pin the windows builds to numpy 1.x until the #warning issue can be addressed. Well, all but the py3.12 windows job. Apparently that #warning was added earlier than I expected, effecting all numpy wheels uploaded for py3.12. So I think that specific combination will have to wait for a fix to numpy.

AlexanderWells-diamond commented 2 months ago

@mdavidsaver Just to confirm, do you want to wait for a Numpy release that fixes that particular issue, or are you happy if we try and work around it?

mdavidsaver commented 2 months ago

At this point the numpy fix for MSVC is merged, but not released. I have re-triggered all jobs to see if they still pass. If so, then I think I am ok with disabling/ignoring the one failing job until the next numpy release.

mdavidsaver commented 2 months ago

Merged as of b1052f4886ccf65d5459f58293acb73afe1521fb .