Skip to content
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

Backport PR #55527: BUG fix deprecation of limit and fill_method #55701

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/source/whatsnew/v2.1.2.rst
Expand Up @@ -8,6 +8,14 @@ including other versions of pandas.

{{ header }}

.. ---------------------------------------------------------------------------
.. _whatsnew_212.deprecations:

Deprecations
~~~~~~~~~~~~

- Reverted deprecation of ``fill_method=None`` in :meth:`DataFrame.pct_change`, :meth:`Series.pct_change`, :meth:`DataFrameGroupBy.pct_change`, and :meth:`SeriesGroupBy.pct_change`; the values ``'backfill'``, ``'bfill'``, ``'pad'``, and ``'ffill'`` are still deprecated (:issue:`53491`)

.. ---------------------------------------------------------------------------
.. _whatsnew_212.regressions:

Expand Down
44 changes: 23 additions & 21 deletions pandas/core/generic.py
Expand Up @@ -11579,6 +11579,7 @@ def pct_change(
How to handle NAs **before** computing percent changes.

.. deprecated:: 2.1
All options of `fill_method` are deprecated except `fill_method=None`.

limit : int, default None
The number of consecutive NAs to fill before stopping.
Expand Down Expand Up @@ -11685,32 +11686,33 @@ def pct_change(
APPL -0.252395 -0.011860 NaN
"""
# GH#53491
if fill_method is not lib.no_default or limit is not lib.no_default:
if fill_method not in (lib.no_default, None) or limit is not lib.no_default:
warnings.warn(
"The 'fill_method' and 'limit' keywords in "
f"{type(self).__name__}.pct_change are deprecated and will be "
"removed in a future version. Call "
f"{'bfill' if fill_method in ('backfill', 'bfill') else 'ffill'} "
"before calling pct_change instead.",
"The 'fill_method' keyword being not None and the 'limit' keyword in "
f"{type(self).__name__}.pct_change are deprecated and will be removed "
"in a future version. Either fill in any non-leading NA values prior "
"to calling pct_change or specify 'fill_method=None' to not fill NA "
"values.",
FutureWarning,
stacklevel=find_stack_level(),
)
if fill_method is lib.no_default:
cols = self.items() if self.ndim == 2 else [(None, self)]
for _, col in cols:
mask = col.isna().values
mask = mask[np.argmax(~mask) :]
if mask.any():
warnings.warn(
"The default fill_method='pad' in "
f"{type(self).__name__}.pct_change is deprecated and will be "
"removed in a future version. Call ffill before calling "
"pct_change to retain current behavior and silence this "
"warning.",
FutureWarning,
stacklevel=find_stack_level(),
)
break
if limit is lib.no_default:
cols = self.items() if self.ndim == 2 else [(None, self)]
for _, col in cols:
mask = col.isna().values
mask = mask[np.argmax(~mask) :]
if mask.any():
warnings.warn(
"The default fill_method='pad' in "
f"{type(self).__name__}.pct_change is deprecated and will "
"be removed in a future version. Either fill in any "
"non-leading NA values prior to calling pct_change or "
"specify 'fill_method=None' to not fill NA values.",
FutureWarning,
stacklevel=find_stack_level(),
)
break
fill_method = "pad"
if limit is lib.no_default:
limit = None
Expand Down
25 changes: 14 additions & 11 deletions pandas/core/groupby/groupby.py
Expand Up @@ -5217,7 +5217,7 @@ def diff(
def pct_change(
self,
periods: int = 1,
fill_method: FillnaOptions | lib.NoDefault = lib.no_default,
fill_method: FillnaOptions | None | lib.NoDefault = lib.no_default,
limit: int | None | lib.NoDefault = lib.no_default,
freq=None,
axis: Axis | lib.NoDefault = lib.no_default,
Expand Down Expand Up @@ -5269,23 +5269,26 @@ def pct_change(
goldfish 0.2 0.125
"""
# GH#53491
if fill_method is not lib.no_default or limit is not lib.no_default:
if fill_method not in (lib.no_default, None) or limit is not lib.no_default:
warnings.warn(
"The 'fill_method' and 'limit' keywords in "
f"{type(self).__name__}.pct_change are deprecated and will be "
"removed in a future version. Call "
f"{'bfill' if fill_method in ('backfill', 'bfill') else 'ffill'} "
"before calling pct_change instead.",
"The 'fill_method' keyword being not None and the 'limit' keyword in "
f"{type(self).__name__}.pct_change are deprecated and will be removed "
"in a future version. Either fill in any non-leading NA values prior "
"to calling pct_change or specify 'fill_method=None' to not fill NA "
"values.",
FutureWarning,
stacklevel=find_stack_level(),
)
if fill_method is lib.no_default:
if any(grp.isna().values.any() for _, grp in self):
if limit is lib.no_default and any(
grp.isna().values.any() for _, grp in self
):
warnings.warn(
"The default fill_method='ffill' in "
f"{type(self).__name__}.pct_change is deprecated and will be "
"removed in a future version. Call ffill before calling "
"pct_change to retain current behavior and silence this warning.",
f"{type(self).__name__}.pct_change is deprecated and will "
"be removed in a future version. Either fill in any "
"non-leading NA values prior to calling pct_change or "
"specify 'fill_method=None' to not fill NA values.",
FutureWarning,
stacklevel=find_stack_level(),
)
Expand Down
16 changes: 8 additions & 8 deletions pandas/tests/frame/methods/test_pct_change.py
Expand Up @@ -29,7 +29,7 @@ def test_pct_change_with_nas(
obj = frame_or_series(vals)

msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
f"{type(obj).__name__}.pct_change are deprecated"
)
with tm.assert_produces_warning(FutureWarning, match=msg):
Expand All @@ -46,7 +46,7 @@ def test_pct_change_numeric(self):
pnl.iat[2, 3] = 60

msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"DataFrame.pct_change are deprecated"
)

Expand All @@ -59,12 +59,11 @@ def test_pct_change_numeric(self):

def test_pct_change(self, datetime_frame):
msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"DataFrame.pct_change are deprecated"
)

with tm.assert_produces_warning(FutureWarning, match=msg):
rs = datetime_frame.pct_change(fill_method=None)
rs = datetime_frame.pct_change(fill_method=None)
tm.assert_frame_equal(rs, datetime_frame / datetime_frame.shift(1) - 1)

rs = datetime_frame.pct_change(2)
Expand Down Expand Up @@ -110,7 +109,7 @@ def test_pct_change_periods_freq(
self, datetime_frame, freq, periods, fill_method, limit
):
msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"DataFrame.pct_change are deprecated"
)

Expand Down Expand Up @@ -144,11 +143,12 @@ def test_pct_change_with_duplicated_indices(fill_method):
{0: [np.nan, 1, 2, 3, 9, 18], 1: [0, 1, np.nan, 3, 9, 18]}, index=["a", "b"] * 3
)

warn = None if fill_method is None else FutureWarning
msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"DataFrame.pct_change are deprecated"
)
with tm.assert_produces_warning(FutureWarning, match=msg):
with tm.assert_produces_warning(warn, match=msg):
result = data.pct_change(fill_method=fill_method)

if fill_method is None:
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/groupby/transform/test_transform.py
Expand Up @@ -1063,7 +1063,7 @@ def test_pct_change(frame_or_series, freq, periods, fill_method, limit):
expected = expected.to_frame("vals")

msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
f"{type(gb).__name__}.pct_change are deprecated"
)
with tm.assert_produces_warning(FutureWarning, match=msg):
Expand Down
15 changes: 9 additions & 6 deletions pandas/tests/series/methods/test_pct_change.py
Expand Up @@ -11,12 +11,11 @@
class TestSeriesPctChange:
def test_pct_change(self, datetime_series):
msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"Series.pct_change are deprecated"
)

with tm.assert_produces_warning(FutureWarning, match=msg):
rs = datetime_series.pct_change(fill_method=None)
rs = datetime_series.pct_change(fill_method=None)
tm.assert_series_equal(rs, datetime_series / datetime_series.shift(1) - 1)

rs = datetime_series.pct_change(2)
Expand Down Expand Up @@ -69,7 +68,7 @@ def test_pct_change_periods_freq(
self, freq, periods, fill_method, limit, datetime_series
):
msg = (
"The 'fill_method' and 'limit' keywords in "
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"Series.pct_change are deprecated"
)

Expand Down Expand Up @@ -101,8 +100,12 @@ def test_pct_change_with_duplicated_indices(fill_method):
# GH30463
s = Series([np.nan, 1, 2, 3, 9, 18], index=["a", "b"] * 3)

msg = "The 'fill_method' and 'limit' keywords in Series.pct_change are deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):
warn = None if fill_method is None else FutureWarning
msg = (
"The 'fill_method' keyword being not None and the 'limit' keyword in "
"Series.pct_change are deprecated"
)
with tm.assert_produces_warning(warn, match=msg):
result = s.pct_change(fill_method=fill_method)

expected = Series([np.nan, np.nan, 1.0, 0.5, 2.0, 1.0], index=["a", "b"] * 3)
Expand Down