-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Implement fillna(..., limit=x) for EAs #58249
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
Conversation
pandas/core/arrays/base.py
Outdated
@@ -1073,6 +1066,12 @@ def fillna( | |||
Length: 6, dtype: Int64 | |||
""" | |||
mask = self.isna() | |||
if limit is not None and limit < len(self): | |||
modify = mask.cumsum() > limit |
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.
self.isna()
can return an ExtensionArray, which may not implement cumsum
because we don't require it to implement _accumulate
(but do require _reduce
). Would it be okay to require _accumulate
? Still not sure about the comparison >
in that case.
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.
@jbrockmendel - any thoughts here?
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’d be fine with that. Could add a comment to that effect
@@ -149,6 +149,16 @@ def test_fillna_frame(self): | |||
"""We treat dictionaries as a mapping in fillna, not a scalar.""" | |||
super().test_fillna_frame() | |||
|
|||
@pytest.mark.xfail(reason="fill value is a dictionary, takes incorrect code path") |
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 cannot use fillna with JSONArray because DataFrame.fillna has special logic for dict / Series argument.
Thanks @rhshadrach |
* BUG: Implement fillna(..., limit=x) for EAs * Comments * type ignores * Update doc/source/whatsnew/v3.0.0.rst --------- Co-authored-by: Matthew Roeschke <[email protected]>
fillna(..., limit=x)
for EAs #58001 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.