Add support for Numpy 2.x (take 2) #442

maffoo · 2024-08-31T18:48:18Z

This builds on the work on @aMarcireau in #429. Per the suggestion from @davidhewitt (#435 (comment)), I've rebased it on main to pick up CI fixes and made some changes to incorporate the latest review comments from @adamreichold.

By default, the extension is compile with support for numpy 1 and 2 (with runtime checks to pick the right binary offset where needed). Features or fields that are specific to a version are hidden by default. Users can opt-out of numpy 1 + numpy 2 by disabling default features and selecting a version. The library panics if the runtime version does not match the compilation version if only one version is selected.

Essentially a port of https://github.com/numpy/numpy/blob/97356bc6f0d6538389a9eef475d883a0f4024c2a/numpy/_core/include/numpy/npy_2_compat.h

…propriate

@stinodego

Contributed by @stinodego

…n bounds for the MSRV build.

davidhewitt · 2024-08-31T20:36:57Z

Thank you! @adamreichold do you expect to have capacity to review this in the near future, or should I try to find some time?

juntyr · 2024-09-20T05:02:35Z

gentle ping @adamreichold @davidhewitt - do you have capacity to review

davidhewitt

Sorry for the delay. I had kept this on my TODO list having not seen a reply here from @adamreichold, but sickness has slowed me down all over the place.

Overall this looks great to me, and it follows all the comments from @adamreichold's review. Thanks very much for moving things along.

I have just a couple of small changes I'd like to see, focussing on the version-specific FFI functions. Given they panic at runtime, I think we should expose a helper is_numpy_2() function so that users can check before calling (and triggering the panic).

Once those are done, if we don't hear from @adamreichold before, I am happy to merge this and carry on with the 0.22 release steps.

davidhewitt · 2024-09-20T05:42:37Z

.github/workflows/ci.yml

+      - name: Edit Cargo.toml and enable new resolver
+        run: |
+          import toml
+          cargo_toml = toml.load("Cargo.toml")
+          cargo_toml["package"]["resolver"] = "2"
+          with open("Cargo.toml", "w") as f:
+            toml.dump(cargo_toml, f)


A separate task, I think we can bump MSRV and remove this.

davidhewitt · 2024-09-20T06:19:56Z

src/npyffi/mod.rs

+    API_VERSION.get_or_init(py, || unsafe {
+        #[allow(non_snake_case)]
+        let PyArray_GetNDArrayCFeatureVersion = api.offset(211) as *const extern "C" fn() -> c_uint;
+        (*PyArray_GetNDArrayCFeatureVersion)()
+    });


This doesn't work if we happen to load the ufunc API first here (we get a nasty crash).

I would prefer to see this moved into a new public function:

Suggested change

API_VERSION.get_or_init(py, || unsafe {

#[allow(non_snake_case)]

let PyArray_GetNDArrayCFeatureVersion = api.offset(211) as *const extern "C" fn() -> c_uint;

(*PyArray_GetNDArrayCFeatureVersion)()

});

fn is_numpy_2() -> bool {

API_VERSION.get_or_init(py, || unsafe { PY_ARRAY_API.PyArray_GetNDArrayCVersion()}) >= API_VERSION_2_0

}

Done. I changed places where we check the version to use this, and re-ordered the conditionals so that we always check if is_numpy_2(py).

davidhewitt · 2024-09-20T06:22:26Z

src/npyffi/mod.rs

    types::{PyAnyMethods, PyCapsule, PyCapsuleMethods, PyModule},
    PyResult, Python,
 };

+pub const API_VERSION_2_0: c_uint = 0x00000012;
+
+pub static API_VERSION: GILOnceCell<c_uint> = GILOnceCell::new();


I think we should not make the direct cache public:

Suggested change

pub static API_VERSION: GILOnceCell<c_uint> = GILOnceCell::new();

static API_VERSION: GILOnceCell<c_uint> = GILOnceCell::new();

davidhewitt · 2024-09-20T06:23:29Z

src/npyffi/mod.rs

+            let api_version = *API_VERSION.get(py).expect("API_VERSION is initialized");
+            if api_version >= API_VERSION_2_0 {
+                panic!(
+                    "{} requires API < {:08X} (NumPy 1) but the runtime version is API {:08X}",
+                    stringify!($fname),
+                    API_VERSION_2_0,
+                    api_version,
+                )
+            }


With new helper:

Suggested change

let api_version = *API_VERSION.get(py).expect("API_VERSION is initialized");

if api_version >= API_VERSION_2_0 {

panic!(

"{} requires API < {:08X} (NumPy 1) but the runtime version is API {:08X}",

stringify!($fname),

API_VERSION_2_0,

api_version,

)

}

assert!(

!is_numpy_2(),

"{} requires API < {:08X} (NumPy 1) but the runtime version is API {:08X}",

stringify!($fname),

API_VERSION_2_0,

API_VERSION.get(py).expect("API_VERSION is initialized),

)

davidhewitt · 2024-09-20T06:23:55Z

src/npyffi/mod.rs

+            let api_version = *API_VERSION.get(py).expect("API_VERSION is initialized");
+            if api_version < API_VERSION_2_0 {
+                panic!(
+                    "{} requires API {:08X} or greater (NumPy 2) but the runtime version is API {:08X}",


Same as above, let's change this to an assert!(is_numpy_2(), ...)

davidhewitt

This looks good to me, many thanks! 🙏

src/npyffi/mod.rs

davidhewitt · 2024-09-23T20:54:46Z

Ah, there is a test failure. It looks like the assertion might just need updating?

maffoo · 2024-09-23T21:48:05Z

Ah, there is a test failure. It looks like the assertion might just need updating?

Fixed the test; since we run this test against both numpy2 and numpy1 I made the npyffi::is_numpy_2 function public so it could be used by the test.

juntyr · 2024-09-24T11:39:47Z

Ah, there is a test failure. It looks like the assertion might just need updating?

Fixed the test; since we run this test against both numpy2 and numpy1 I made the npyffi::is_numpy_2 function public so it could be used by the test.

Did you already push your fix?

maffoo · 2024-09-24T14:32:23Z

Ah, there is a test failure. It looks like the assertion might just need updating?

Fixed the test; since we run this test against both numpy2 and numpy1 I made the npyffi::is_numpy_2 function public so it could be used by the test.

Did you already push your fix?

Oops, looks like I forgot; pushed now. Thanks for catching that.

davidhewitt · 2024-09-25T21:17:59Z

Hmm, we have a crash on 32 bit windows. This makes me think an FFI definition might be slightly wrong somewhere (using i32 instead of c_long or similar...)

davidhewitt · 2024-09-26T07:09:16Z

I'm happy to try to help debug this, but it might be 1-2 weeks before I can find a good debugging session for this. There's a lot of competing priorities for me right now personally and also in PyO3 between the Python 3.13 freethreaded release and various auxiliary projects.

maffoo · 2024-09-30T15:57:42Z

Hmm, we have a crash on 32 bit windows. This makes me think an FFI definition might be slightly wrong somewhere (using i32 instead of c_long or similar...)

I've been trying to debug the issue on my fork where I can run CI: maffoo#2. One problem is that that when using numpy 2 on a 32-bit windows build, the PyDataType_ELSIZE function is returning 0 in places where it shouldn't, while when using numpy 1 on a 32-bit windows build the ELSIZE is correct so cargo tests pass but there is a crash when loading a compiled python extension. I'm still looking into what is going on, but haven't been able to track it down.

davidhewitt · 2024-09-30T19:57:24Z

Thank you. I just took a long stare at the headers too, and really don't see anything obvious going on. I can hope to make time later in the week or so to also try to debug.

itamarst · 2024-10-01T17:58:31Z

Just out of curiosity, is supporting 32-bit Windows actually that important at this point? Some data points on usage:

https://www.pcbenchmarks.net/os-marketshare.html (last graph)
https://usercontent.irccloud-cdn.com/file/ykGZWTQW/pyca_cryptography_rage_dashboard.pdf - cryptography package usage; this is a different set of Python users, but still indicative.

https://discuss.python.org/t/consider-downgrading-windows-32-bit-from-tier-1-to-tier-2-or-tier-3-in-python-3-13/33719/24 has some relevant discussion.

(There's also "open an issue, merge this, and move on, and see if anyone cares", rather than explicitly saying you won't support it.)

itamarst · 2024-10-01T18:01:46Z

Adding to above data points, latest release of SciPy doesn't appear to have 32-bit Windows builds. Conda has dropped 32-bit Windows as well.

davidhewitt · 2024-10-01T19:59:38Z

I think that's a pragmatic option and while it's not ideal, I would be happy to for the moment set it up so that 32-bit windows will fail to compile and point at an issue. That way we can get feedback on whether it's worth fixing.

Moving ahead with merge would also allow us to get on with 0.22 release in parallel with investigating this problem.

Also in a similar vein I just opened #445

maffoo · 2024-10-03T18:39:01Z

I think that's a pragmatic option and while it's not ideal, I would be happy to for the moment set it up so that 32-bit windows will fail to compile and point at an issue. That way we can get feedback on whether it's worth fixing.

Moving ahead with merge would also allow us to get on with 0.22 release in parallel with investigating this problem.

I've disabled compilation on 32-bit windows for now. I used compile_error! to do this but let me know if there's an alternative you would prefer. I filed #448 to track investigating this and potentially re-enabling support for 32-bit windows.

I also removed 32-bit windows from the ci; I had to add some setup-python actions to get things to run on my fork, as it seems that ubuntu-latest has updated to python 3.12 and now fails with an error about being in an externally managed environment if you try to pip install anything without being in a virtualenv. I was seeing the same error even with setup-python if using python 3.12, so I set things up for python 3.11 for now (this seems like a problem in setup-python itself). I put up #447 to address the ci issues separately since it also looks like ubuntu-latest no longer supports python 3.7, so we could do that first rather than lumping everything in here.

One other change: I renamed two structs to _PyArray_DescrNumPy2 and _PyArray_LegacyDescr to match what is in the numpy c source, reverting the change following the @adamreichold's comment. I think following the names in the numpy source is worth it, especially since there are some thorny ABI issues with 32-bit windows and having to translate struct names while tracking these down was making things more difficult.

davidhewitt · 2024-10-03T19:57:00Z

Perfect, CI all green! Thanks so much for all the help here 🎉

As a final step, could you please add a CHANGELOG entry to this PR? Then let's merge and proceed!

maffoo · 2024-10-03T20:34:43Z

As a final step, could you please add a CHANGELOG entry to this PR? Then let's merge and proceed!

Done.

juntyr · 2024-10-06T23:59:38Z

rust-numpy/README.md

Line 128 in 505a79c

    
           on a range of semver-incompatible versions, currently `>= 0.13, < 0.16`. Cargo does not

in the README still needs to be updated

aMarcireau and others added 15 commits August 31, 2024 08:58

Remove outdated 1.0 functions, add new 2.0 functions

3b6c0b7

Essentially a port of https://github.com/numpy/numpy/blob/97356bc6f0d6538389a9eef475d883a0f4024c2a/numpy/_core/include/numpy/npy_2_compat.h

Use the new "universal" access functions

c2e55f8

Fix runtime tests

e060735

Remove feature flags and always check the version at runtime where ap…

31924fb

…propriate

Avoid API changes by using self.py()

7f59548

Fixup

9d7e0a2

Fix flags for Windows

3777299

Contributed by @stinodego

Avoid half-open range patterns to appease our MSRV build.

c89932b

Remove residual numpy-1/2 feature usage.

92b0947

Use explicit patterns so exhaustiveness checking works, but avoid ope…

9de97ba

…n bounds for the MSRV build.

Add ci job to test numpy2 and allow numpy2 in examples

be3ce9b

Make size and buf fields public in npy_static_string

1a93b96

Fixes from @adamreichold review

2882c6a

Simplify impl_api macro

61fb9e0

maffoo mentioned this pull request Aug 31, 2024

Add support for Numpy 2.x #429

Closed

lint

17eda7f

lpsinger mentioned this pull request Sep 10, 2024

Support of numpy 2.0 cds-astro/mocpy#145

Closed

davidhewitt reviewed Sep 20, 2024

View reviewed changes

Fixes from review

abebd0f

maffoo requested a review from davidhewitt September 20, 2024 19:03

davidhewitt approved these changes Sep 23, 2024

View reviewed changes

davidhewitt reviewed Sep 23, 2024

View reviewed changes

src/npyffi/mod.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

da0807f

maffoo requested a review from davidhewitt September 23, 2024 21:48

Fix dtypes tests to work with numpy 1 or 2

6013343

maffoo mentioned this pull request Sep 30, 2024

Update pyo3 to 0.22.0 (without "py-clone") #435

Merged

12 tasks

abstractqqq mentioned this pull request Oct 2, 2024

Update to Polars >=0.43 Once Rust NumPy upgrades to Pyo3 >= 0.22 abstractqqq/polars_ds_extension#265

Open

Rename to _PyArray_DescrNumPy2 and _PyArray_LegacyDescr to match c code

5dd5280

maffoo mentioned this pull request Oct 3, 2024

Numpy2 support fails when running with 32-bit windows #448

Closed

Fail compilation on 32-bit windows

4322102

kylebarron mentioned this pull request Oct 3, 2024

Feature/h3ronpy worldbank/DECAT_Space2Stats#75

Merged

Add a changelog entry

d09a35d

davidhewitt merged commit f397117 into PyO3:main Oct 4, 2024
46 of 48 checks passed

nmandery mentioned this pull request Oct 4, 2024

Support numpy 2 nmandery/h3ronpy#61

Closed

prehner mentioned this pull request Oct 6, 2024

Update to PyO3 0.22 itt-ustutt/num-dual#80

Merged

kylebarron mentioned this pull request Oct 6, 2024

update to geoarrow 0.3. fixes #56 nmandery/h3ronpy#59

Merged

kylebarron mentioned this pull request Oct 7, 2024

Read Arrow C Stream from Arrow PyCapsule Interface vega/vegafusion#501

Closed

itamarst mentioned this pull request Oct 11, 2024

Support for Numpy 2 #409

Closed

kylebarron mentioned this pull request Oct 14, 2024

Remove 32-bit windows wheel builds kylebarron/arro3#238

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Numpy 2.x (take 2) #442

Add support for Numpy 2.x (take 2) #442

maffoo commented Aug 31, 2024

davidhewitt commented Aug 31, 2024

juntyr commented Sep 20, 2024

davidhewitt left a comment

davidhewitt Sep 20, 2024

davidhewitt Sep 20, 2024

maffoo Sep 20, 2024

davidhewitt Sep 20, 2024

maffoo Sep 20, 2024

davidhewitt Sep 20, 2024

maffoo Sep 20, 2024

davidhewitt Sep 20, 2024

maffoo Sep 20, 2024

davidhewitt left a comment

davidhewitt commented Sep 23, 2024

maffoo commented Sep 23, 2024

juntyr commented Sep 24, 2024

maffoo commented Sep 24, 2024

davidhewitt commented Sep 25, 2024

davidhewitt commented Sep 26, 2024

maffoo commented Sep 30, 2024

davidhewitt commented Sep 30, 2024

itamarst commented Oct 1, 2024

itamarst commented Oct 1, 2024

davidhewitt commented Oct 1, 2024 •

edited

Loading

maffoo commented Oct 3, 2024

davidhewitt commented Oct 3, 2024

maffoo commented Oct 3, 2024

juntyr commented Oct 6, 2024

	pub static API_VERSION: GILOnceCell<c_uint> = GILOnceCell::new();
	static API_VERSION: GILOnceCell<c_uint> = GILOnceCell::new();

Add support for Numpy 2.x (take 2) #442

Add support for Numpy 2.x (take 2) #442

Conversation

maffoo commented Aug 31, 2024

davidhewitt commented Aug 31, 2024

juntyr commented Sep 20, 2024

davidhewitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidhewitt left a comment

Choose a reason for hiding this comment

davidhewitt commented Sep 23, 2024

maffoo commented Sep 23, 2024

juntyr commented Sep 24, 2024

maffoo commented Sep 24, 2024

davidhewitt commented Sep 25, 2024

davidhewitt commented Sep 26, 2024

maffoo commented Sep 30, 2024

davidhewitt commented Sep 30, 2024

itamarst commented Oct 1, 2024

itamarst commented Oct 1, 2024

davidhewitt commented Oct 1, 2024 • edited Loading

maffoo commented Oct 3, 2024

davidhewitt commented Oct 3, 2024

maffoo commented Oct 3, 2024

juntyr commented Oct 6, 2024

davidhewitt commented Oct 1, 2024 •

edited

Loading