Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.2k
(fix): handle internal type promotion andnas 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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
757b0ca to951c217CompareUnable to reproduce at least the 3.10 test failures on an ubuntu 22 machine |
| if isinstance(array_or_dtype, str | bytes): | ||
| if should_return_str_or_bytes: | ||
| return array_or_dtype | ||
| return type(array_or_dtype) |
There was a problem hiding this comment.
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 returntype(array_or_dtype) in the case of extension array promotion because it is handled internally by the extension array functionality
There was a problem hiding this comment.
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
| raise ValueError( | ||
| f"Cannot cast values to shared type, found values: {arrays_and_dtypes}" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The main difference betweenhttps://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.
There was a problem hiding this comment.
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.
@deeplycloudy@richard-berg I think this is ready for review |
whats-new.rstapi.rst