ENH: EA.fillna copy=True #53728
ENH: EA.fillna copy=True #53728
Conversation
pandas/core/arrays/base.py
Outdated
| @@ -802,12 +809,20 @@ def fillna( | |||
| npvalues = self.astype(object) | |||
| func(npvalues, limit=limit, mask=mask) | |||
| new_values = self._from_sequence(npvalues, dtype=self.dtype) | |||
| if not copy: | |||
There was a problem hiding this comment.
I don't like this, this is actually slower than copy=True
There was a problem hiding this comment.
agreed this is not good. would you prefer to just ignore copy in this case?
| new_values[mask] = value | ||
| else: | ||
| new_values = self.copy() | ||
| if not copy: | ||
| new_values = self[:] |
There was a problem hiding this comment.
Why do we need a shallow copy in these cases?
There was a problem hiding this comment.
i figured on principle to return a new object, doesnt make much difference
There was a problem hiding this comment.
How does this compare with other EA methods?
There was a problem hiding this comment.
i see a couple of return self and a couple of return self[:] in the base class methods. so not too consistent
There was a problem hiding this comment.
I don't care then, but we should make this consistent (not in this pr)
There was a problem hiding this comment.
im leaning towards consistently returning self as in the Block method we may want to determine whether a copy was made and result is not values is an easy way to do that.
mroeschke
added
Missing-data
|
Updated to ignore "copy" in the base class fillna method in ffill/bfill cases. Added a warning+test for 3rd party EAs that dont yet have the copy keyword in the signature. |
|
i think comments have been addressed |
|
Sorry I thought that we had already merged this. Will merge when CI is green |
|
thx @jbrockmendel |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.cc @phofl for comments on the CoW handling in Block code
cc @jorisvandenbossche for thoughts on deprecation path for 3rd party EAs
Edit: also worth discussing the "author's discretion" on respecting copy=False.