Skip to content

Regression in DataArrays created from Pandas #10301

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

Open
5 tasks
richard-berg opened this issue May 9, 2025 · 9 comments · May be fixed by #10304
Open
5 tasks

Regression in DataArrays created from Pandas #10301

richard-berg opened this issue May 9, 2025 · 9 comments · May be fixed by #10304
Labels
bug needs triage Issue that has not been reviewed by xarray team member

Comments

@richard-berg
Copy link

richard-berg commented May 9, 2025

What happened?

Given:

index1 = np.array([1, 2, 3])
index2 = np.array([1, 2, 4])
srs = pd.Series(index=index1, data=1).convert_dtypes()
arr = srs.to_xarray()

Now consider:

>>> arr.reindex(index=index2)

In xarray 2023.1.0 this gave a reasonable (if weakly-typed) result.

<xarray.DataArray (index: 3)>
array([1, 1, nan], dtype=object)
Coordinates:
  * index    (index) int64 1 2 4

While upgrading to xarray 2025.3.x + pandas 2.x, my colleagues found it now raises:

TypeError: Cannot interpret 'Int64Dtype()' as a data type

What did you expect to happen?

Ideally, the result would be:

<xarray.DataArray (index: 3)> Size: 27B
PandasExtensionArray(array=<IntegerArray>
[1, 1, <NA>]
Length: 3, dtype: Int64)
Coordinates:
  * index    (index) <U1 12B '1' '2' '4'

Minimal Complete Verifiable Example

import numpy as np
import pandas as pd
import xarray as xr
index1 = np.array([1, 2, 3])
index2 = np.array([1, 2, 4])
srs = pd.Series(index=index1, data=1).convert_dtypes()
arr = srs.to_xarray()
arr.reindex(index=index2)

MVCE confirmation

  • Minimal example — the example is as focused as reasonably possible to demonstrate the underlying issue in xarray.
  • Complete example — the example is self-contained, including all data and the text of any traceback.
  • Verifiable example — the example copy & pastes into an IPython prompt or Binder notebook, returning the result.
  • New issue — a search of GitHub Issues suggests this is not a duplicate.
  • Recent environment — the issue occurs with the latest version of xarray and its dependencies.

Anything else we need to know?

The difference is that arr.dtype is now pd.Int64Dtype() rather than np.dtype("object"), thanks to #8723. While arguably an improvement in typing, the xarray core doesn't seem ready to handle the former. In this case, core.dtypes.maybe_promote() is blindly passing a Pandas dtype to np.issubdtype, oops.

Patching this immediate issue is more revealing: reindex then fails when duck_array_ops.where(condition, x, y) tries to coerce x & y to a common dtype. The new extension-array code in as_shared_dtype is not at all general: when y is a scalar (the fill_value from the reindex operation), it simply gives up.

Once I understood the cause of the reindex issue above, producing more -- and much more worrisome -- failures was trivial:

>>> arr + 5

TypeError: unsupported operand type(s) for +: 'PandasExtensionArray' and 'int'

>>> np.add(arr, 5)

TypeError: 'PandasExtensionArray' object is not callable

>>> arr.fillna(0)

AttributeError: 'int' object has no attribute 'dtype'

I'd venture to say that the pandas df.to_xarray() / srs.to_xarray() methods have become foot-guns, bordering on unusable, now that pandas 2.x has reimplemented all of its native datatypes on top of ExtensionArray / ExtensionDtype.

The good news is I have a fix. The bad news is it's pretty invasive, needing careful oversight from someone who actually knows what they're doing. (Before this week I'd never used xarray, nor looked at the numpy / pandas source code.)

For now I might recommend excluding ALL numeric dtypes from being promoted to duck arrays, similar to what #9042 did for datetimes. (Basically everything except Categoricals, which seem to be the one extension type with good coverage in the xarray test suite, and which don't support the vast majority of ufuncs regardless.) That would at least allow people to safely continue using to_xarray() on modern versions of pandas, though you'd lose all the speed & type safety that @ilan-gold worked to achieve in 2024.5 & onward.

Environment

INSTALLED VERSIONS

commit: None
python: 3.12.10 | packaged by conda-forge | (main, Apr 10 2025, 22:21:13) [GCC 13.3.0]
python-bits: 64
OS: Linux
OS-release: 4.18.0-372.32.1.el8_6.x86_64
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: ('en_US', 'UTF-8')
libhdf5: 1.14.2
libnetcdf: None

xarray: 2025.3.1
pandas: 2.2.3
numpy: 1.26.4
scipy: 1.15.2
netCDF4: None
pydap: None
h5netcdf: 1.6.1
h5py: 3.9.0
zarr: 3.0.6
cftime: None
nc_time_axis: None
iris: None
bottleneck: 1.4.2
dask: 2025.3.0
distributed: 2025.3.0
matplotlib: 3.10.1
cartopy: None
seaborn: 0.13.2
numbagg: 0.9.0
fsspec: 2024.9.0
cupy: 13.4.0
pint: None
sparse: 0.16.0
flox: None
numpy_groupies: None
setuptools: 78.1.0
pip: 25.0.1
conda: None
pytest: 8.3.5
mypy: 1.15.0
IPython: 8.35.0
sphinx: None

@richard-berg richard-berg added bug needs triage Issue that has not been reviewed by xarray team member labels May 9, 2025
@ilan-gold
Copy link
Contributor

ilan-gold commented May 9, 2025

@richard-berg thanks for the issue.

import numpy as np
import pandas as pd
import xarray as xr
index1 = np.array([1, 2, 3])
index2 = np.array([1, 2, 4])
srs = pd.Series(index=index1, data=1).convert_dtypes()
arr = srs.to_xarray()
arr + 5

works for me on main maybe because of #10278. But I'm not sure it's the same as your example since you have x there in the addition example.

As for the text coverage, I agree we should increase it. I will add the cases you raise here but since you appear to be aware of more, I would love more guidance. If you look through my PRs here, it's a bit of whack-a-mole because while I think I am using relatively sound practices as I go, xarray has a lot of edge cases in its API that I am not familiar with. If you're aware of some, it would be great to handle them.

I would be opposed to somehow going around special casing because now we actually do let through datetimes as well as interval arrays (which are both tested even if it is not immediately obvious). The reason I had to do #9042 was exactly because all of the special casing that existed before was so complex that unraveling it required a massive PR.

So with special casing, we would be looking at categoricals, interval types, and datetimes passed through and only numerics excluded, until another type comes along. So I'd somewhat rather be very clear here that everything passes through.

@ilan-gold
Copy link
Contributor

P.S I see:

TypeError: 'PandasExtensionArray' object is not callable

And we just made it so that PandasExtensionArray is no longer part of the public API. So hopefully this condition is not something that can be run into

@ilan-gold ilan-gold linked a pull request May 9, 2025 that will close this issue
4 tasks
@dcherian
Copy link
Contributor

dcherian commented May 9, 2025

Whoops, int float and string should be converted to the numpy types. Looks like we lost this somehow and aren't testing it :(

@ilan-gold
Copy link
Contributor

While I could see floats being a good idea, nullable integers do not exist in numpy:

import numpy as np


In [3]: np.array([np.nan, 1, 2], dtype="int32")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[3], line 1
----> 1 np.array([np.nan, 1, 2], dtype="int32")

ValueError: cannot convert float NaN to integer

@richard-berg
Copy link
Author

richard-berg commented May 9, 2025

Thanks for checking, and for the quick fillna patch! FYI I do have a robust fix that handles all of the above in a general way, while preserving strong types (including nullable ints). Just may take a few days to get legal approval for (and work thru the mechanical details of) exfiltrating code off my network.

@dcherian
Copy link
Contributor

dcherian commented May 9, 2025

nullable integers do not exist in numpy:

I know but this is what we do with numpy masked arrays today.

Ideally we'd convert to https://github.com/mdhaber/marray in the near future.

Regardless, for now we'd like to enable any "extra" dtypes (categorical, intervals) while using the numpy dtypes as much as we can.

@ilan-gold
Copy link
Contributor

ilan-gold commented May 9, 2025

Ideally we'd convert to https://github.com/mdhaber/marray in the near future.

Interesting!

@richard-berg I would be interested why not just limit this behavior to from_dataframe i.e., users who wish to convert into an xarray object from pandas will lose the type but going the other way would be disabled. We rely on integer arrays and boolean arrays in https://github.com/scverse/anndata and projects like https://geopandas.org/en/stable/ rely on geometry extension types. So users constructing xarray objects should be allowed to continue to do that, no? And then they can exit xarray with their types in tact?

@ilan-gold
Copy link
Contributor

Regardless, for now we'd like to enable any "extra" dtypes (categorical, intervals) while using the numpy dtypes as much as we can.

+1 here given the above example of geometry which is not in pandas core but is widely used int he geospatial community.

@ilan-gold
Copy link
Contributor

I should say @richard-berg been thinking about this since you posted and I really appreciate your contribution and effort here btw! Looking forward to seeing you PR :)))) I love to "users" get involved with open source, wish it was easier given corporate situations sometimes. Thanks a million again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs triage Issue that has not been reviewed by xarray team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants