Skip to content

(fix): handle internal type promotion and nas for extension arrays properly #10423

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

(fix): handle internal type promotion and nas for extension arrays properly #10423

wants to merge 17 commits into from

Conversation

ilan-gold
Copy link
Contributor

@github-actions github-actions bot added topic-backends topic-indexing topic-documentation topic-CF conventions topic-groupby topic-dask topic-plotting topic-testing topic-performance topic-cftime topic-zarr Related to zarr storage library topic-arrays related to flexible array support CI Continuous Integration tools topic-html-repr topic-rolling dependencies Pull requests that update a dependency file Automation Github bots, testing workflows, release automation topic-DataTree Related to the implementation of a DataTree class topic-typing topic-hypothesis Strategies or tests using the hypothesis library io topic-NamedArray Lightweight version of Variable labels Jun 16, 2025
@ilan-gold ilan-gold force-pushed the ig-minimumal-reindex branch from 757b0ca to 951c217 Compare June 18, 2025 16:26
@ilan-gold ilan-gold marked this pull request as ready for review June 26, 2025 12:05
@ilan-gold
Copy link
Contributor Author

Unable to reproduce at least the 3.10 test failures on an ubuntu 22 machine

ilan-gold
ilan-gold commented Jun 26, 2025
View reviewed changes
xarray/core/dtypes.py
Comment on lines +248 to +251
if isinstance(array_or_dtype, str | bytes):
if should_return_str_or_bytes:
return array_or_dtype
return type(array_or_dtype)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually do not want to produce return type(array_or_dtype) in the case of extension array promotion because it is handled internally by the extension array functionality

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, this function should be the same

ilan-gold
ilan-gold commented Jun 26, 2025
View reviewed changes
xarray/core/extension_array.py
Comment on lines +149 to +151
raise ValueError(
f"Cannot cast values to shared type, found values: {arrays_and_dtypes}"
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main difference between https://github.com/pydata/xarray/pull/10380/files#diff-c803294f5216cbbdffa30f0b0c9f16a7e39855d4dd309c88d654bc317a78adc0R136-R137 and mine is the raising of the error here instead of using internal pandas functionality. I think this is reasonable to throw an error here, but maybe others have a better non-private function(ality) we could use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other difference is we revert to the old concat behavior, which I think was reasonable before.

@ilan-gold
Copy link
Contributor Author

@deeplycloudy @richard-berg I think this is ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Automation Github bots, testing workflows, release automation CI Continuous Integration tools dependencies Pull requests that update a dependency file io topic-arrays related to flexible array support topic-backends topic-CF conventions topic-cftime topic-dask topic-DataTree Related to the implementation of a DataTree class topic-documentation topic-groupby topic-html-repr topic-hypothesis Strategies or tests using the hypothesis library topic-indexing topic-NamedArray Lightweight version of Variable topic-performance topic-plotting topic-rolling topic-testing topic-typing topic-zarr Related to zarr storage library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression in DataArrays created from Pandas
2 participants
@ilan-gold @richard-berg