Skip to content

Commit

Permalink
Backport PR pandas-dev#55527: BUG fix deprecation of limit and `fil…
Browse files Browse the repository at this point in the history
…l_method` in `pct_change`
  • Loading branch information
Charlie-XIAO authored and rhshadrach committed Oct 26, 2023
1 parent 0a44346 commit 9dfb53c
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 47 deletions.
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

0 comments on commit 9dfb53c

Please sign in to comment.