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

Variable may contain numpy scalars with numpy>=2.1 #9399

Open
keewis opened this issue Aug 22, 2024 · 4 comments · May be fixed by #9403
Open

Variable may contain numpy scalars with numpy>=2.1 #9399

keewis opened this issue Aug 22, 2024 · 4 comments · May be fixed by #9403
Labels

Comments

@keewis
Copy link
Collaborator

keewis commented Aug 22, 2024

What is your issue?

I'm not sure if this is a bad thing or not, but while writing tests @TomNicholas and I noticed that starting with numpy=2.1, Variable objects may contain numpy scalars, especially as the result of an aggregation like mean.

This is caused by numpy scalars gaining a __array_namespace__ method, which is then interpreted by as_compatible_data as an actual array.

To fix this, we could change

if not isinstance(data, np.ndarray) and (
hasattr(data, "__array_function__") or hasattr(data, "__array_namespace__")
):
to

if not isinstance(data, (np.numeric, np.ndarray)) and ( 
    hasattr(data, "__array_function__") or hasattr(data, "__array_namespace__") 
):

but not sure if that's worth it.

To reproduce, try this in an environment with numpy>=2.1:

import numpy as np
import xarray as xr

v = xr.Variable((), np.float64(4.1))
repr(v)
# <xarray.Variable ()> Size: 8B
# np.float64(4.1)
@keewis keewis added needs triage Issue that has not been reviewed by xarray team member bug and removed needs triage Issue that has not been reviewed by xarray team member labels Aug 22, 2024
@dcherian
Copy link
Contributor

Aren't there non-numeric scalars? Perhaps .ndim == 0 is a better check.

@shoyer
Copy link
Member

shoyer commented Aug 26, 2024

I think we should probably add a special case for NumPy scalars, to cast them to arrays. It's simpler for Xarray users to always have NumPy arrays.

@keewis
Copy link
Collaborator Author

keewis commented Aug 26, 2024

Wouldn't

not isinstance(data, (np.generic, np.ndarray))

work then? With that scalars would be passed through to the np.asarray call, just like np.ndarray.

@shoyer
Copy link
Member

shoyer commented Aug 26, 2024

Wouldn't

not isinstance(data, (np.generic, np.ndarray))

work then? With that scalars would be passed through to the np.asarray call, just like np.ndarray.

Yes, that would do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants