Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds copy parameter to __array__ for numpy 2.0 #9393

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

andrew-s28
Copy link
Contributor

@andrew-s28 andrew-s28 commented Aug 21, 2024

xarray has been raising some deprecation warnings whenever np.array is called on an xr.DataArray. This is the same message as documented in #9312, but the issue is not specifically related to the interpolate_na method. Instead, the issue can be produced when using NumPy>=2.0.0 as:

>>> import xarray as xr
>>> import numpy as np
>>> da = xr.DataArray([0])
>>> np.array(da)
DeprecationWarning: __array__ implementation doesn't accept a copy keyword, so passing copy=False failed. __array__ must implement 'dtype' and 'copy' keyword arguments.

It is caused by the AbstractArray.__array__ method not containing a copy kwarg that NumPy 2.0 now expects.

A fix was proposed in #9312 by @kmuehlbauer, but upon testing it I found that this fix is not backwards compatible when using NumPy versions<2.0:

>>> import xarray as xr 
>>> import numpy as np
>>> da = xr.DataArray([0])
>>> np.array(da)
ValueError: NoneType copy mode not allowed.

The only way I found that would be able to address the deprecation warning and maintain backwards compatibility was to use a conditional in the AbstractArray.__array__ method based on the installed NumPy version, which is how this PR is structured (based on the suggested implementation from the migration guide, from this SciPy PR).

Using a conditional based on a version doesn't feel great, but the only other option I see is just ignoring the copy the kwarg argument that is passed into the __array__ method, which is ignoring user input and also isn't great. I'd be interested to see if there are any more elegant ways to deal with this deprecation warning, though.

That said, if xarray ever drops support for NumPy<2.0, then this can be reverted to the more straightforward fix proposed in #9312.

Copy link

welcome bot commented Aug 21, 2024

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

@max-sixty
Copy link
Collaborator

Thanks @andrew-s28, this looks good.

Could we add a test that there's no warning?

@andrew-s28
Copy link
Contributor Author

andrew-s28 commented Aug 22, 2024

@max-sixty test is now included (and seems to be passing).

I accidentally synced my feature branch (instead of main) with the upstream and it goofed up the commit history a bit, so my apologies. I seem to be making it worse trying to rebase, so let me know if there's anything I should do to fix it up.

Think I have it cleaned up now. Sorry about that, this was the first time where merges to the upstream came in after my PR that I had to manage. Some lessons learned. 👍

@max-sixty max-sixty merged commit a04d857 into pydata:main Aug 22, 2024
28 checks passed
Copy link

welcome bot commented Aug 22, 2024

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! celebration gif

@max-sixty
Copy link
Collaborator

Thanks a lot!

(No prob re commits — it all gets squashed on merge anyway!)

dcherian added a commit to TomNicholas/xarray that referenced this pull request Aug 22, 2024
* main: (214 commits)
  Adds copy parameter to __array__ for numpy 2.0 (pydata#9393)
  `numpy 2` compatibility in the `pydap` backend (pydata#9391)
  pyarrow dependency added to doc environment (pydata#9394)
  Extend padding functionalities (pydata#9353)
  refactor GroupBy internals (pydata#9389)
  Combine `UnsignedIntegerCoder` and `CFMaskCoder` (pydata#9274)
  passing missing parameters to ZarrStore.open_store when opening a datatree (pydata#9377)
  Fix tests on big-endian systems (pydata#9380)
  Improve error message on `ds['x', 'y']` (pydata#9375)
  Improve error message for missing coordinate index (pydata#9370)
  Add flaky to TestNetCDF4ViaDaskData (pydata#9373)
  Make chunk manager an option in `set_options` (pydata#9362)
  Revise (pydata#9371)
  Remove duplicate word from docs (pydata#9367)
  Adding open_groups to BackendEntryPointEngine, NetCDF4BackendEntrypoint, and H5netcdfBackendEntrypoint (pydata#9243)
  Revise (pydata#9366)
  Fix rechunking to a frequency with empty bins. (pydata#9364)
  whats-new entry for dropping python 3.9 (pydata#9359)
  drop support for `python=3.9` (pydata#8937)
  Revise (pydata#9357)
  ...
dcherian added a commit to dcherian/xarray that referenced this pull request Aug 30, 2024
* main:
  Adds copy parameter to __array__ for numpy 2.0 (pydata#9393)
  `numpy 2` compatibility in the `pydap` backend (pydata#9391)
  pyarrow dependency added to doc environment (pydata#9394)
  Extend padding functionalities (pydata#9353)
  refactor GroupBy internals (pydata#9389)
  Combine `UnsignedIntegerCoder` and `CFMaskCoder` (pydata#9274)
  passing missing parameters to ZarrStore.open_store when opening a datatree (pydata#9377)
  Fix tests on big-endian systems (pydata#9380)
  Improve error message on `ds['x', 'y']` (pydata#9375)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storing results of interpolate_na into numpy.array
2 participants