zarr-developers / zarr-python

An implementation of chunked, compressed, N-dimensional arrays for Python.
https://zarr.readthedocs.io
MIT License
1.45k stars 273 forks source link

Fix Array.__array__ for numpy 2.1 #2106

Closed dstansby closed 2 weeks ago

dstansby commented 3 weeks ago

This updates array for numpy 2.1, specifically by implementing the copy keyword argument.

The one test failure is an issue with a new version of numcodecs (0.13.0) which is a simple fix, but easier to make when we bump the numocdecs version in the test dependencies, so I'd advocate for merging this PR and dealing with the numcodecs bump in a follow up PR.

TODO:

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.98%. Comparing base (c5c4698) to head (c88a6c7). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2106 +/- ## ========================================== - Coverage 99.98% 99.98% -0.01% ========================================== Files 38 38 Lines 14714 14711 -3 ========================================== - Hits 14712 14709 -3 Misses 2 2 ``` | [Files](https://app.codecov.io/gh/zarr-developers/zarr-python/pull/2106?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zarr-developers) | Coverage Δ | | |---|---|---| | [zarr/core.py](https://app.codecov.io/gh/zarr-developers/zarr-python/pull/2106?src=pr&el=tree&filepath=zarr%2Fcore.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zarr-developers#diff-emFyci9jb3JlLnB5) | `100.00% <100.00%> (ø)` | |
joshmoore commented 3 weeks ago

The only immediate question in my mind is whether this adds any additional copies of the array.

dstansby commented 3 weeks ago

Using the test script below on my laptop shows that with numpy 2.0.1:

So behaviour with this PR is the same as before.

import numpy as np
import zarr

z = zarr.zeros((100, 100), chunks=(10, 10), dtype='i4')
print("Copy not set")
print("Same memory?", id(z[:]) == id(np.array(z)))
jhamman commented 3 weeks ago

@dstansby - have you looked into the test failure in the minimum_build. Certainly looks unrelated but something we should probably fix.

dstansby commented 2 weeks ago

Yes, in the top comment here I said:

The one test failure is an issue with a new version of numcodecs (0.13.0) which is a simple fix, but easier to make when we bump the numocdecs version in the test dependencies, so I'd advocate for merging this PR and dealing with the numcodecs bump in a follow up PR.

joshmoore commented 2 weeks ago

Trusting @dstansby to be Mr. Fixit! :smile: