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

BUG fix deprecation of limit and fill_method in pct_change #55527

Merged
merged 12 commits into from
Oct 26, 2023
44 changes: 23 additions & 21 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -11712,6 +11712,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 @@ -11818,32 +11819,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
Original file line number Diff line number Diff line change
Expand Up @@ -5286,7 +5286,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 @@ -5338,23 +5338,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
26 changes: 14 additions & 12 deletions pandas/tests/frame/methods/test_pct_change.py
Original file line number Diff line number Diff line change
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,12 +143,15 @@ 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
)

msg = (
"The 'fill_method' and 'limit' keywords in "
"DataFrame.pct_change are deprecated"
)
with tm.assert_produces_warning(FutureWarning, match=msg):
result = data.pct_change(fill_method=fill_method)
if fill_method is not None:
msg = (
"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):
result = data.pct_change(fill_method=fill_method)
else:
result = data.pct_change(fill_method=None)
Charlie-XIAO marked this conversation as resolved.
Show resolved Hide resolved

if fill_method is None:
second_column = [np.nan, np.inf, np.nan, np.nan, 2.0, 1.0]
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/groupby/transform/test_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,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
19 changes: 12 additions & 7 deletions pandas/tests/series/methods/test_pct_change.py
Original file line number Diff line number Diff line change
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,9 +100,15 @@ 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):
result = s.pct_change(fill_method=fill_method)
if fill_method is not None:
msg = (
"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):
result = s.pct_change(fill_method=fill_method)
else:
result = s.pct_change()
Charlie-XIAO marked this conversation as resolved.
Show resolved Hide resolved

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