-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
BUG: Take method of NumpyExtensionArray now returns another extension array with the correct dtype. #62502
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
base: main
Are you sure you want to change the base?
BUG: Take method of NumpyExtensionArray now returns another extension array with the correct dtype. #62502
Conversation
…take method to return correct dtype.
|
Several tests are currently failing due to a new version of |
| np.int64, | ||
| ], | ||
| ) | ||
| def test_take_assigns_integer_dtype_when_fill_disallowed(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.
this test is unnecessary
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.
I added this test to make sure that take does the expected thing when not allowed to fill missing values. I did a quick search, but I didn't find any other tests for this. There are other tests that work with this method, but I would like to have a test explicitly for this behavior to check for regressions and make it clear that take should preserve dtype if it can. If you still think this test isn't needed, can you explain why you think so?
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.
this is tested in the extension tests
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.
@aijams if you address this we can get this merged
In the edge case when called with integer arrays and asked to access non-existent entries (to be replaced with NaN), the
takemethod ofNumpyExtensionArrayproduces arrays whose dtypes don't match their underlying data.Specifically,
takepromotes the underlying data to a floating-point type, but doesn't promote the dtype of the extension array to match.These changes ensure that the result of this method has the correct dtype for its data.
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.