-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: EA.fillna copy=True #53728
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
ENH: EA.fillna copy=True #53728
Changes from all commits
d680398
a3b79c0
98fafd7
0b355ba
34731b1
d5786c3
c09fd75
89f95a2
3252158
dff594f
4039e2a
b8a8248
14a207e
51af605
71738bd
3d21ea4
59f35ea
3446312
656d96c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -872,6 +872,7 @@ def fillna( | |
value: object | ArrayLike | None = None, | ||
method: FillnaOptions | None = None, | ||
limit: int | None = None, | ||
copy: bool = True, | ||
) -> Self: | ||
""" | ||
Fill NA/NaN values using the specified method. | ||
|
@@ -896,6 +897,14 @@ def fillna( | |
maximum number of entries along the entire axis where NaNs will be | ||
filled. | ||
|
||
copy : bool, default True | ||
Whether to make a copy of the data before filling. If False, then | ||
the original should be modified and no new memory should be allocated. | ||
For ExtensionArray subclasses that cannot do this, it is at the | ||
author's discretion whether to ignore "copy=False" or to raise. | ||
The base class implementation ignores the keyword in pad/backfill | ||
cases. | ||
|
||
Returns | ||
------- | ||
ExtensionArray | ||
|
@@ -932,10 +941,16 @@ def fillna( | |
return self[::-1].take(indexer, allow_fill=True) | ||
else: | ||
# fill with value | ||
new_values = self.copy() | ||
if not copy: | ||
new_values = self[:] | ||
else: | ||
new_values = self.copy() | ||
new_values[mask] = value | ||
else: | ||
new_values = self.copy() | ||
if not copy: | ||
new_values = self[:] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a shallow copy in these cases? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i figured on principle to return a new object, doesnt make much difference There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this compare with other EA methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see a couple of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't care then, but we should make this consistent (not in this pr) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. im leaning towards consistently returning |
||
else: | ||
new_values = self.copy() | ||
return new_values | ||
|
||
def dropna(self) -> Self: | ||
|
Uh oh!
There was an error while loading. Please reload this page.