-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
BUG: Add limit_area to EA ffill/bfill #56616
Changes from 7 commits
60aa82f
27b0c57
fd44a52
2033b59
0f4e400
da58e4b
9e53011
ab863d1
8271d68
09cccb8
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 |
---|---|---|
|
@@ -70,6 +70,7 @@ | |
unique, | ||
) | ||
from pandas.core.array_algos.quantile import quantile_with_mask | ||
from pandas.core.missing import _fill_limit_area_1d | ||
from pandas.core.sorting import ( | ||
nargminmax, | ||
nargsort, | ||
|
@@ -954,7 +955,12 @@ def interpolate( | |
) | ||
|
||
def _pad_or_backfill( | ||
self, *, method: FillnaOptions, limit: int | None = None, copy: bool = True | ||
self, | ||
*, | ||
method: FillnaOptions, | ||
limit: int | None = None, | ||
limit_area: Literal["inside", "outside"] | None = None, | ||
copy: bool = True, | ||
) -> Self: | ||
""" | ||
Pad or backfill values, used by Series/DataFrame ffill and bfill. | ||
|
@@ -1012,6 +1018,12 @@ def _pad_or_backfill( | |
DeprecationWarning, | ||
stacklevel=find_stack_level(), | ||
) | ||
if limit_area is not None: | ||
raise NotImplementedError( | ||
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. We only get here if 3rd party authors didn't implement this themselves, correct? 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. Right - this is only hit when EA author overrides |
||
f"{type(self).__name__} does not implement limit_area " | ||
"(added in pandas 2.2). 3rd-party ExtnsionArray authors " | ||
"need to add this argument to _pad_or_backfill." | ||
) | ||
return self.fillna(method=method, limit=limit) | ||
|
||
mask = self.isna() | ||
|
@@ -1021,6 +1033,8 @@ def _pad_or_backfill( | |
meth = missing.clean_fill_method(method) | ||
|
||
npmask = np.asarray(mask) | ||
if limit_area is not None and not npmask.all(): | ||
_fill_limit_area_1d(npmask, limit_area) | ||
if meth == "pad": | ||
indexer = libalgos.get_fill_indexer(npmask, limit=limit) | ||
return self.take(indexer, allow_fill=True) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -192,7 +192,12 @@ def __getitem__(self, item: PositionalIndexer) -> Self | Any: | |
return self._simple_new(self._data[item], newmask) | ||
|
||
def _pad_or_backfill( | ||
self, *, method: FillnaOptions, limit: int | None = None, copy: bool = True | ||
self, | ||
*, | ||
method: FillnaOptions, | ||
limit: int | None = None, | ||
limit_area: Literal["inside", "outside"] | None = None, | ||
copy: bool = True, | ||
) -> Self: | ||
mask = self._mask | ||
|
||
|
@@ -204,7 +209,21 @@ def _pad_or_backfill( | |
if copy: | ||
npvalues = npvalues.copy() | ||
new_mask = new_mask.copy() | ||
elif limit_area is not None: | ||
mask = mask.copy() | ||
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. Non-blocking, but why do we need this copy here? |
||
func(npvalues, limit=limit, mask=new_mask) | ||
|
||
if limit_area is not None and not mask.all(): | ||
mask = mask.T | ||
neg_mask = ~mask | ||
first = neg_mask.argmax() | ||
last = len(neg_mask) - neg_mask[::-1].argmax() - 1 | ||
if limit_area == "inside": | ||
new_mask[:first] |= mask[:first] | ||
new_mask[last + 1 :] |= mask[last + 1 :] | ||
elif limit_area == "outside": | ||
new_mask[first + 1 : last] |= mask[first + 1 : last] | ||
|
||
if copy: | ||
return self._simple_new(npvalues.T, new_mask.T) | ||
else: | ||
|
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.
3rd party instead of 3rd part?
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 would put this into the ExtensionArray section as well