BUG: Fix dtypes change using replace with nullable dtypes (#40732) #40864
Conversation
|
Hello @hasan-yaman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-04-12 18:30:53 UTC |
pandas/core/internals/blocks.py
Outdated
| nb = self.coerce_to_target_dtype(value) | ||
| if isinstance( | ||
| self.values, (IntegerArray, FloatingArray) | ||
| ) and self._can_hold_element(value): |
There was a problem hiding this comment.
This seems like a very specific check, can you comment on why it is necessary / makes sense? Also, how does BooleanArray fit in here? Do we have tests on if replacement for BooleanArray works as expected?
There was a problem hiding this comment.
I'd guess the same underlying problem for nullable dtypes is also responsible for #40732 (comment), so it would be great if the patch were able to apply more generally and fix a case like that as well (unless you think the underlying issues are different).
There was a problem hiding this comment.
I focused the problem encounters in IntegerArray and FloatingArray that's why I added the check. But, i think i should focus on nullable arrays.
I think for the replace method, we don't have test that cover all the nullable arrays. Will try to add some.
| [ | ||
| ( | ||
| [1.0, 2.0, 3.999, 4.4], | ||
| {1.0: 9}, |
There was a problem hiding this comment.
Since different paths are exercised by .replace(1.0, 9.0) and .replace({1.0, 9.0}), can the test be parameterized such that those cases can also be tested at the same time?
mzeitlin11
added
Bug
NA - MaskedArrays
There was a problem hiding this comment.
Thanks for the pr @hasan-yaman! Left some comments, plus can you please add a whatsnew entry for 1.3?
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
|
@hasan-yaman can you add a release note |
|
Thanks @hasan-yaman for the PR. closing as stale. ping if you want to continue. |
replacewith nullable dtypes #40732