Revert "reverting"

This reverts commit 2cb449b.
pandas-dev · mroeschke · Jun 9, 2023 · Jun 4, 2023 · Jun 4, 2023 · Jun 4, 2023
commit 446569458dcff158f8fb804c03a6d0ebfbb152fc
diff --git a/pandas/core/generic.py b/pandas/core/generic.py
@@ -11144,7 +11144,7 @@ def pct_change(
         self,
         periods: int = 1,
         fill_method: FillnaOptions | None | lib.NoDefault = lib.no_default,
-        limit: int | None = None,
+        limit: int | None | lib.NoDefault = lib.no_default,
         freq=None,
         **kwargs,
     ) -> Self:
@@ -11275,21 +11275,20 @@ def pct_change(
         GOOG  0.179241  0.094112   NaN
         APPL -0.252395 -0.011860   NaN
         """
-        # GH#53491: deprecate default fill_method="pad"
-        # TODO: In 3.x, change default fill_method=None, then also in 3.x
-        # deprecate the fill_method and limit keywords, and finally remove
-        # them in 4.x
-        if fill_method is lib.no_default:
+        if fill_method is not lib.no_default or limit is not lib.no_default:
+            # GH#53491
             warnings.warn(
-                f"The default fill_method='pad' in {type(self).__name__}.pct_change "
-                "is deprecated and will be changed to None in a future version of "
-                "pandas. Pass fill_method='pad' to retain current behavior or "
-                "fill_method=None to adopt the future default and silence this "
-                "warning.",
+                "The 'fill_method' and 'limit' keywords in "
+                f"{type(self).__name__}.pct_change are deprecated and will be "
+                "removed in a future version. Call fillna directly before "
+                "calling pct_change instead.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
+        if fill_method is lib.no_default:
             fill_method = "pad"
+        if limit is lib.no_default:
+            limit = None
 
         axis = self._get_axis_number(kwargs.pop("axis", "index"))
         if fill_method is None:

@@ -3946,7 +3946,7 @@ def pct_change(
         self,
         periods: int = 1,
         fill_method: FillnaOptions | lib.NoDefault = lib.no_default,
-        limit: int | None = None,
+        limit: int | None | lib.NoDefault = lib.no_default,
         freq=None,
         axis: Axis | lib.NoDefault = lib.no_default,
     ):
@@ -3958,21 +3958,20 @@ def pct_change(
         Series or DataFrame
             Percentage changes within each group.
         """
-        # GH#53491: deprecate default fill_method="ffill"
-        # TODO: In 3.x, change default fill_method=None, then also in 3.x
-        # deprecate the fill_method and limit keywords, and finally remove
-        # them in 4.x
-        if fill_method is lib.no_default:
+        if fill_method is not lib.no_default or limit is not lib.no_default:
+            # GH#53491
             warnings.warn(
-                f"The default fill_method='ffill' in {type(self).__name__}.pct_change "
-                "is deprecated and will be changed to None in a future version of "
-                "pandas. Pass fill_method='ffill' to retain current behavior or "
-                "fill_method=None to adopt the future default and silence this "
-                "warning.",
+                "The 'fill_method' and 'limit' keywords in "
+                f"{type(self).__name__}.pct_change are deprecated and will be "
+                "removed in a future version. Call fillna directly before "
+                "calling pct_change instead.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
+        if fill_method is lib.no_default:
             fill_method = "ffill"
+        if limit is lib.no_default:
+            limit = None
 
         if axis is not lib.no_default:
             axis = self.obj._get_axis_number(axis)

diff --git a/pandas/tests/frame/methods/test_pct_change.py b/pandas/tests/frame/methods/test_pct_change.py
@@ -10,7 +10,7 @@
 
 class TestDataFramePctChange:
     @pytest.mark.parametrize(
-        "periods,fill_method,limit,exp",
+        "periods, fill_method, limit, exp",
         [
             (1, "ffill", None, [np.nan, np.nan, np.nan, 1, 1, 1.5, 0, 0]),
             (1, "ffill", 1, [np.nan, np.nan, np.nan, 1, 1, 1.5, 0, np.nan]),
@@ -28,7 +28,12 @@ def test_pct_change_with_nas(
         vals = [np.nan, np.nan, 1, 2, 4, 10, np.nan, np.nan]
         obj = frame_or_series(vals)
 
-        res = obj.pct_change(periods=periods, fill_method=fill_method, limit=limit)
+        msg = (
+            "The 'fill_method' and 'limit' keywords in "
+            f"{type(obj).__name__}.pct_change are deprecated"
+        )
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            res = obj.pct_change(periods=periods, fill_method=fill_method, limit=limit)
         tm.assert_equal(res, frame_or_series(exp))
 
     def test_pct_change_numeric(self):
@@ -40,41 +45,49 @@ def test_pct_change_numeric(self):
         pnl.iat[1, 1] = np.nan
         pnl.iat[2, 3] = 60
 
+        msg = (
+            "The 'fill_method' and 'limit' keywords in "
+            "DataFrame.pct_change are deprecated"
+        )
+
         for axis in range(2):
             expected = pnl.ffill(axis=axis) / pnl.ffill(axis=axis).shift(axis=axis) - 1
-            result = pnl.pct_change(axis=axis, fill_method="pad")
+
+            with tm.assert_produces_warning(FutureWarning, match=msg):
+                result = pnl.pct_change(axis=axis, fill_method="pad")
             tm.assert_frame_equal(result, expected)
 
     def test_pct_change(self, datetime_frame):
-        msg = "The default fill_method='pad' in DataFrame.pct_change 
8000
is deprecated"
+        msg = (
+            "The 'fill_method' and 'limit' keywords in "
+            "DataFrame.pct_change are deprecated"
+        )
 
-        rs = datetime_frame.pct_change(fill_method=None)
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            rs = datetime_frame.pct_change(fill_method=None)
         tm.assert_frame_equal(rs, datetime_frame / datetime_frame.shift(1) - 1)
 
-        with tm.assert_produces_warning(FutureWarning, match=msg):
-            rs = datetime_frame.pct_change(2)
+        rs = datetime_frame.pct_change(2)
         filled = datetime_frame.fillna(method="pad")
         tm.assert_frame_equal(rs, filled / filled.shift(2) - 1)
 
-        rs = datetime_frame.pct_change(fill_method="bfill", limit=1)
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            rs = datetime_frame.pct_change(fill_method="bfill", limit=1)
         filled = datetime_frame.fillna(method="bfill", limit=1)
         tm.assert_frame_equal(rs, filled / filled.shift(1) - 1)
 
-        with tm.assert_produces_warning(FutureWarning, match=msg):
-            rs = datetime_frame.pct_change(freq="5D")
+        rs = datetime_frame.pct_change(freq="5D")
         filled = datetime_frame.fillna(method="pad")
         tm.assert_frame_equal(
             rs, (filled / filled.shift(freq="5D") - 1).reindex_like(filled)
         )
 
     def test_pct_change_shift_over_nas(self):
         s = Series([1.0, 1.5, np.nan, 2.5, 3.0])
-        df = DataFrame({"a": s, "b": s})
 
-        msg = "The default fill_method='pad' in DataFrame.pct_change is deprecated"
-        with tm.assert_produces_warning(FutureWarning, match=msg):
-            chg = df.pct_change()
+        df = DataFrame({"a": s, "b": s})
 
+        chg = df.pct_change()
         expected = Series([np.nan, 0.5, 0.0, 2.5 / 1.5 - 1, 0.2])
         edf = DataFrame({"a": expected, "b": expected})
         tm.assert_frame_equal(chg, edf)
@@ -93,18 +106,31 @@ def test_pct_change_shift_over_nas(self):
     def test_pct_change_periods_freq(
         self, datetime_frame, freq, periods, fill_method, limit
     ):
-        # GH#7292
-        rs_freq = datetime_frame.pct_change(
-            freq=freq, fill_method=fill_method, limit=limit
-        )
-        rs_periods = datetime_frame.pct_change(
-            periods, fill_method=fill_method, limit=limit
+        msg = (
+            "The 'fill_method' and 'limit' keywords in "
+            "DataFrame.pct_change are deprecated"
         )
+
+        # GH#7292
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            rs_freq = datetime_frame.pct_change(
+                freq=freq, fill_method=fill_method, limit=limit
+            )
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            rs_periods = datetime_frame.pct_change(
+                periods, fill_method=fill_method, limit=limit
+            )
         tm.assert_frame_equal(rs_freq, rs_periods)
 
         empty_ts = DataFrame(index=datetime_frame.index, columns=datetime_frame.columns)
-        rs_freq = empty_ts.pct_change(freq=freq, fill_method=fill_method, limit=limit)
-        rs_periods = empty_ts.pct_change(periods, fill_method=fill_method, limit=limit)
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            rs_freq = empty_ts.pct_change(
+                freq=freq, fill_method=fill_method, limit=limit
+            )
+        with tm.assert_produces_warning(FutureWarning, match=msg):
+            rs_periods = empty_ts.pct_change(
+                periods, fill_method=fill_method, limit=limit
+            )
         tm.assert_frame_equal(rs_freq, rs_periods)
 
 
@@ -114,7 +140,13 @@ def test_pct_change_with_duplicated_indices(fill_method):
     data = DataFrame(
         {0: [np.nan, 1, 2, 3, 9, 18], 1: [0, 1, np.nan, 3, 9, 18]}, index=["a", "b"] * 3
     )
-    result = data.pct_change(fill_method=fill_method)
+
+    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 None:
         second_column = [np.nan, np.inf, np.nan, np.nan, 2.0, 1.0]

diff --git a/pandas/tests/groupby/transform/test_transform.py b/pandas/tests/groupby/transform/test_transform.py
@@ -169,18 +169,8 @@ def test_transform_axis_1(request, transformation_func):
     msg = "DataFrame.groupby with axis=1 is deprecated"
     with tm.assert_produces_warning(FutureWarning, match=msg):
         gb = df.groupby([0, 0, 1], axis=1)
-
-    pct_change_msg = (
-        "The default fill_method='ffill' in DataFrameGroupBy.pct_change is deprecated"
-    )
-    if transformation_func == "pct_change":
-        with tm.assert_produces_warning(FutureWarning, match=pct_change_msg):
-            result = gb.transform("pct_change", *args)
-        with tm.assert_produces_warning(FutureWarning, match=pct_change_msg):
-            expected = df.T.groupby([0, 0, 1]).transform("pct_change", *args).T
-    else:
-        result = gb.transform(transformation_func, *args)
-        expected = df.T.groupby([0, 0, 1]).transform(transformation_func, *args).T
+    result = gb.transform(transformation_func, *args)
+    expected = df.T.groupby([0, 0, 1]).transform(transformation_func, *args).T
 
     if transformation_func in ["diff", "shift"]:
         # Result contains nans, so transpose coerces to float
@@ -414,25 +404,11 @@ def mock_op(x):
         test_op = lambda x: x.transform(transformation_func)
         mock_op = lambda x: getattr(x, transformation_func)()
 
-    msg = "The default fill_method='pad' in DataFrame.pct_change is deprecated"
-    groupby_msg = (
-        "The default fill_method='ffill' in DataFrameGroupBy.pct_change is deprecated"
-    )
-
-    if transformation_func == "pct_change":
-        with tm.assert_produces_warning(FutureWarning, match=groupby_msg):
-            result = test_op(df.groupby("A"))
-    else:
-        result = test_op(df.groupby("A"))
-
+    result = test_op(df.groupby("A"))
     # pass the group in same order as iterating `for ... in df.groupby(...)`
     # but reorder to match df's index since this is a transform
     groups = [df[["B"]].iloc[4:6], df[["B"]].iloc[6:], df[["B"]].iloc[:4]]
-    if transformation_func == "pct_change":
-        with tm.assert_produces_warning(FutureWarning, match=msg):
-            expected = concat([mock_op(g) for g in groups]).sort_index()
-    else:
-        expected = concat([mock_op(g) for g in groups]).sort_index()
+    expected = concat([mock_op(g) for g in groups]).sort_index()
     # sort_index does not preserve the freq
     expected = expected.set_axis(df.index)
 
@@ -993,9 +969,14 @@ def test_pct_change(frame_or_series, freq, periods, fill_method, limit):
     else:
         expected = expected.to_frame("vals")
 
-    result = gb.pct_change(
-        periods=periods, fill_method=fill_method, limit=limit, freq=freq
+    msg = (
+        "The 'fill_method' and 'limit' keywords in "
+        f"{type(gb).__name__}.pct_change are deprecated"
     )
+    with tm.assert_produces_warning(FutureWarning, match=msg):
+        result = gb.pct_change(
+            periods=periods, fill_method=fill_method, limit=limit, freq=freq
+        )
     tm.assert_equal(result, expected)
 
 
@@ -1413,21 +1394,13 @@ def test_null_group_str_transformer(request, dropna, transformation_func):
     args = get_groupby_method_args(transformation_func, df)
     gb = df.groupby("A", dropna=dropna)
 
-    msg = "The default fill_method='pad' in DataFrame.pct_change is deprecated"
-    groupby_msg = (
-        "The default fill_method='ffill' in DataFrameGroupBy.pct_change is deprecated"
-    )
-
     buffer = []
     for k, (idx, group) in enumerate(gb):
         if transformation_func == "cumcount":
             # DataFrame has no cumcount method
             res = DataFrame({"B": range(len(group))}, index=group.index)
         elif transformation_func == "ngroup":
             res = DataFrame(len(group) * [k], index=group.index, columns=["B"])
-        elif transformation_func == "pct_change":
-            with tm.assert_produces_warning(FutureWarning, match=msg):
-                res = getattr(group[["B"]], "pct_change")(*args)
         else:
             res = getattr(group[["B"]], transformation_func)(*args)
         buffer.append(res)
@@ -1440,11 +1413,7 @@ def test_null_group_str_transformer(request, dropna, transformation_func):
         # ngroup/cumcount always returns a Series as it counts the groups, not values
         expected = expected["B"].rename(None)
 
-    if transformation_func == "pct_change":
-        with tm.assert_produces_warning(FutureWarning, match=groupby_msg):
-            result = gb.transform("pct_change", *args)
-    else:
-        result = gb.transform(transformation_func, *args)
+    result = gb.transform(transformation_func, *args)
 
     tm.assert_equal(result, expected)
 
@@ -1496,21 +1465,13 @@ def test_null_group_str_transformer_series(dropna, transformation_func):
     args = get_groupby_method_args(transformation_func, ser)
     gb = ser.groupby([1, 1, np.nan], dropna=dropna)
 
-    msg = "The default fill_method='pad' in Series.pct_change is deprecated"
-    groupby_msg = (
-        "The default fill_method='ffill' in SeriesGroupBy.pct_change is deprecated"
-    )
-
     buffer = []
     for k, (idx, group) in enumerate(gb):
         if transformation_func == "cumcount":
             # Series has no cumcount method
             res = Series(range(len(group)), index=group.index)
         elif transformation_func == "ngroup":
             res = Series(k, index=group.index)
-        elif transformation_func == "pct_change":
-            with tm.assert_produces_warning(FutureWarning, match=msg):
-                res = getattr(group, "pct_change")(*args)
         else:
             res = getattr(group, transformation_func)(*args)
         buffer.append(res)
@@ -1519,10 +1480,7 @@ def test_null_group_str_transformer_series(dropna, transformation_func):
         buffer.append(Series([np.nan], index=[3], dtype=dtype))
     expected = concat(buffer)
 
-    if transformation_func == "pct_change":
-        with tm.assert_produces_warning(FutureWarning, match=groupby_msg):
-            result = gb.transform("pct_change", *args)
-    else:
+    with tm.assert_produces_warning(None):
         result = gb.transform(transformation_func, *args)
 
     tm.assert_equal(result, expected)
@@ -1565,14 +1523,6 @@ def test_as_index_no_change(keys, df, groupby_func):
     args = get_groupby_method_args(groupby_func, df)
     gb_as_index_true = df.groupby(keys, as_index=True)
     gb_as_index_false = df.groupby(keys, as_index=False)
-
-    msg = "The default fill_method='ffill' in DataFrameGroupBy.pct_change is deprecated"
-    if groupby_func == "pct_change":
-        with tm.assert_produces_warning(FutureWarning, match=msg):
-            result = gb_as_index_true.transform("pct_change", *args)
-        with tm.assert_produces_warning(FutureWarning, match=msg):
-            expected = gb_as_index_false.transform("pct_change", *args)
-    else:
-        result = gb_as_index_true.transform(groupby_func, *args)
-        expected = gb_as_index_false.transform(groupby_func, *args)
+    result = gb_as_index_true.transform(groupby_func, *args)
+    expected = gb_as_index_false.transform(groupby_func, *args)
     tm.assert_equal(result, expected)