From cc8d7459137c78b4e9bb1ddf068e734c6250a23a Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Wed, 4 Oct 2023 15:26:27 +0800
Subject: [PATCH 1/8] extra specific deprecation messages

---
 pandas/core/generic.py         | 60 ++++++++++++++++++++-----
 pandas/core/groupby/groupby.py | 81 ++++++++++++++++++++++++++--------
 2 files changed, 113 insertions(+), 28 deletions(-)

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index 738f4cbe6bc43..a68d79826f5eb 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -11694,6 +11694,7 @@ def pct_change(
             How to handle NAs **before** computing percent changes.
 
             .. deprecated:: 2.1
+                `fill_method` is deprecated except `fill_method=None`.
 
         limit : int, default None
             The number of consecutive NAs to fill before stopping.
@@ -11799,18 +11800,60 @@ def pct_change(
         GOOG  0.179241  0.094112   NaN
         APPL -0.252395 -0.011860   NaN
         """
-        # GH#53491
-        if fill_method is not lib.no_default or limit is not lib.no_default:
+        # GH#53491: deprecate the `fill_method` and `limit` keyword, except
+        # `fill_method=None` that does not fill missing values
+        if fill_method not in (lib.no_default, None) and limit is not lib.no_default:
+            # `fill_method` in FillnaOptions and `limit` is specified
             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"fill_method={fill_method} and the limit keyword 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'}"
+                f"(limit={limit}) before calling pct_change instead.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
+        elif fill_method not in (lib.no_default, None):
+            # `fill_method` in FillnaOptions and `limit` is not specified
+            warnings.warn(
+                f"fill_method={fill_method} in {type(self).__name__}.pct_change is "
+                "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.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
-        if fill_method is lib.no_default:
+            limit = None
+        elif fill_method is None and limit is not lib.no_default:
+            # `fill_method` is None but `limit` is specified
+            warnings.warn(
+                f"The limit keyword in {type(self).__name__}.pct_change is deprecated "
+                "and will be removed in a future version. It does not have any effect "
+                "when using with fill_method=None. Remove it to silence this warning.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
+        elif fill_method is None:
+            # `fill_method` is None and `limit` is not specified: this should not be
+            # deprecated. TODO(3.x) allow only `fill_method=None` and raise on anything
+            # else; deprecate the `fill_method` keyword entirely. TODO(4.x) remove the
+            # `fill_method` keyword.
+            # https://github.com/pandas-dev/pandas/issues/53491#issuecomment-1728443050
+            limit = None
+        elif limit is not lib.no_default:
+            # `fill_method` is not specified but `limit` is specified
+            warnings.warn(
+                "The default fill_method='pad' and the limit keyword in "
+                f"{type(self).__name__}.pct_change are deprecated and will be removed "
+                f"in a future version. Call ffill(limit={limit}) before calling "
+                "pct_change to retain the current behavior and silence this warning.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
+            fill_method = "pad"
+        else:
+            # `fill_method` and `limit` are both not specified
+            # GH#54981: avoid unnecessary FutureWarning
             cols = self.items() if self.ndim == 2 else [(None, self)]
             for _, col in cols:
                 mask = col.isna().values
@@ -11825,10 +11868,7 @@ def pct_change(
                         FutureWarning,
                         stacklevel=find_stack_level(),
                     )
-                    break
-            fill_method = "pad"
-        if limit is lib.no_default:
-            limit = None
+            fill_method, limit = "pad", None
 
         axis = self._get_axis_number(kwargs.pop("axis", "index"))
         if fill_method is None:
diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py
index a022bfd1bd9bc..41d47c5730dcc 100644
--- a/pandas/core/groupby/groupby.py
+++ b/pandas/core/groupby/groupby.py
@@ -5282,7 +5282,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,
@@ -5333,30 +5333,75 @@ def pct_change(
          catfish    NaN    NaN
         goldfish    0.2  0.125
         """
-        # GH#53491
-        if fill_method is not lib.no_default or limit is not lib.no_default:
+        # GH#53491: deprecate the `fill_method` and `limit` keyword, except
+        # `fill_method=None` that does not fill missing values
+        if fill_method not in (lib.no_default, None) and limit is not lib.no_default:
+            # `fill_method` in FillnaOptions and `limit` is specified
             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"fill_method={fill_method} and the limit keyword 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'}"
+                f"(limit={limit}) before calling pct_change instead.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
+        elif fill_method not in (lib.no_default, None):
+            # `fill_method` in FillnaOptions and `limit` is not specified
+            warnings.warn(
+                f"fill_method={fill_method} in {type(self).__name__}.pct_change is "
+                "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.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
-        if fill_method is lib.no_default:
-            if 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.",
-                    FutureWarning,
-                    stacklevel=find_stack_level(),
-                )
-            fill_method = "ffill"
-        if limit is lib.no_default:
             limit = None
+        elif fill_method is None and limit is not lib.no_default:
+            # `fill_method` is None but `limit` is specified
+            warnings.warn(
+                f"The limit keyword in {type(self).__name__}.pct_change is deprecated "
+                "and will be removed in a future version. It does not have any effect "
+                "when using with fill_method=None. Remove it to silence this warning.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
+        elif fill_method is None:
+            # `fill_method` is None and `limit` is not specified: this should not be
+            # deprecated. TODO(3.x) allow only `fill_method=None` and raise on anything
+            # else; deprecate the `fill_method` keyword entirely. TODO(4.x) remove the
+            # `fill_method` keyword.
+            # https://github.com/pandas-dev/pandas/issues/53491#issuecomment-1728443050
+            limit = None
+        elif limit is not lib.no_default:
+            # `fill_method` is not specified but `limit` is specified
+            warnings.warn(
+                "The default fill_method='pad' and the limit keyword in "
+                f"{type(self).__name__}.pct_change are deprecated and will be removed "
+                f"in a future version. Call ffill(limit={limit}) before calling "
+                "pct_change to retain the current behavior and silence this warning.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
+            fill_method = "pad"
+        else:
+            # `fill_method` and `limit` are both not specified
+            # GH#54981: avoid unnecessary FutureWarning
+            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(),
+                    )
+            fill_method, limit = "pad", None
 
         if axis is not lib.no_default:
             axis = self.obj._get_axis_number(axis)

From afbeac0aea2d360ddcf99f5430fe4e7adf11ce57 Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Wed, 4 Oct 2023 16:34:02 +0800
Subject: [PATCH 2/8] deprecation in docstring

---
 pandas/core/generic.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index a68d79826f5eb..7abe500d82fd3 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -11694,7 +11694,7 @@ def pct_change(
             How to handle NAs **before** computing percent changes.
 
             .. deprecated:: 2.1
-                `fill_method` is deprecated except `fill_method=None`.
+                All options of `fill_method` are deprecated except `fill_method=None`.
 
         limit : int, default None
             The number of consecutive NAs to fill before stopping.

From 096bc7859c1e5d772b055b001fd4f41812b7be3a Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Sun, 15 Oct 2023 16:17:03 +0800
Subject: [PATCH 3/8] use fill_method=None

---
 pandas/core/generic.py         | 39 +++++++++++++++-------------------
 pandas/core/groupby/groupby.py | 39 +++++++++++++++-------------------
 2 files changed, 34 insertions(+), 44 deletions(-)

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index f79d09037edde..639c205e7ffe0 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -11822,22 +11822,22 @@ def pct_change(
         # `fill_method=None` that does not fill missing values
         if fill_method not in (lib.no_default, None) and limit is not lib.no_default:
             # `fill_method` in FillnaOptions and `limit` is specified
+            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
             warnings.warn(
                 f"fill_method={fill_method} and the limit keyword 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'}"
-                f"(limit={limit}) before calling pct_change instead.",
+                f"in a future version. Use obj.{fill_type}(limit={limit}).pct_change"
+                "(fill_method=None) instead.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
         elif fill_method not in (lib.no_default, None):
             # `fill_method` in FillnaOptions and `limit` is not specified
+            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
             warnings.warn(
                 f"fill_method={fill_method} in {type(self).__name__}.pct_change is "
-                "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.",
+                "deprecated and will be removed in a future version. Use "
+                f"obj.{fill_type}().pct_change(fill_method=None) instead.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
@@ -11863,8 +11863,9 @@ def pct_change(
             warnings.warn(
                 "The default fill_method='pad' and the limit keyword in "
                 f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Call ffill(limit={limit}) before calling "
-                "pct_change to retain the current behavior and silence this warning.",
+                f"in a future version. Use obj.ffill(limit={limit}).pct_change"
+                "(fill_method=None) to retain the current behavior and silence this "
+                "warning.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
@@ -11872,20 +11873,14 @@ def pct_change(
         else:
             # `fill_method` and `limit` are both not specified
             # GH#54981: avoid unnecessary FutureWarning
-            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(),
-                    )
+            warnings.warn(
+                "The default fill_method='pad' and the limit keyword in "
+                f"{type(self).__name__}.pct_change are deprecated and will be removed "
+                f"in a future version. Use obj.ffill().pct_change(fill_method=None) "
+                "to retain the current behavior and silence this warning.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
             fill_method, limit = "pad", None
 
         axis = self._get_axis_number(kwargs.pop("axis", "index"))
diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py
index 4b2b00161ba50..579d2b1f3cd92 100644
--- a/pandas/core/groupby/groupby.py
+++ b/pandas/core/groupby/groupby.py
@@ -5341,22 +5341,22 @@ def pct_change(
         # `fill_method=None` that does not fill missing values
         if fill_method not in (lib.no_default, None) and limit is not lib.no_default:
             # `fill_method` in FillnaOptions and `limit` is specified
+            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
             warnings.warn(
                 f"fill_method={fill_method} and the limit keyword 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'}"
-                f"(limit={limit}) before calling pct_change instead.",
+                f"in a future version. Use obj.{fill_type}(limit={limit}).pct_change"
+                "(fill_method=None) instead.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
         elif fill_method not in (lib.no_default, None):
             # `fill_method` in FillnaOptions and `limit` is not specified
+            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
             warnings.warn(
                 f"fill_method={fill_method} in {type(self).__name__}.pct_change is "
-                "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.",
+                "deprecated and will be removed in a future version. Use "
+                f"obj.{fill_type}().pct_change(fill_method=None) instead.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
@@ -5382,8 +5382,9 @@ def pct_change(
             warnings.warn(
                 "The default fill_method='pad' and the limit keyword in "
                 f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Call ffill(limit={limit}) before calling "
-                "pct_change to retain the current behavior and silence this warning.",
+                f"in a future version. Use obj.ffill(limit={limit}).pct_change"
+                "(fill_method=None) to retain the current behavior and silence this "
+                "warning.",
                 FutureWarning,
                 stacklevel=find_stack_level(),
             )
@@ -5391,20 +5392,14 @@ def pct_change(
         else:
             # `fill_method` and `limit` are both not specified
             # GH#54981: avoid unnecessary FutureWarning
-            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(),
-                    )
+            warnings.warn(
+                "The default fill_method='pad' and the limit keyword in "
+                f"{type(self).__name__}.pct_change are deprecated and will be removed "
+                f"in a future version. Use obj.ffill().pct_change(fill_method=None) "
+                "to retain the current behavior and silence this warning.",
+                FutureWarning,
+                stacklevel=find_stack_level(),
+            )
             fill_method, limit = "pad", None
 
         if axis is not lib.no_default:

From e7c6296ea826a6fc0906e529bcfb59d3f1a8a245 Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Sun, 15 Oct 2023 23:01:59 +0800
Subject: [PATCH 4/8] apply richard's suggestions

---
 pandas/core/generic.py         | 84 +++++++++-------------------------
 pandas/core/groupby/groupby.py | 80 +++++++-------------------------
 2 files changed, 38 insertions(+), 126 deletions(-)

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index 639c205e7ffe0..75d24c68a9f85 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -11818,70 +11818,28 @@ def pct_change(
         GOOG  0.179241  0.094112   NaN
         APPL -0.252395 -0.011860   NaN
         """
-        # GH#53491: deprecate the `fill_method` and `limit` keyword, except
-        # `fill_method=None` that does not fill missing values
-        if fill_method not in (lib.no_default, None) and limit is not lib.no_default:
-            # `fill_method` in FillnaOptions and `limit` is specified
-            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
-            warnings.warn(
-                f"fill_method={fill_method} and the limit keyword in "
-                f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Use obj.{fill_type}(limit={limit}).pct_change"
-                "(fill_method=None) instead.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-        elif fill_method not in (lib.no_default, None):
-            # `fill_method` in FillnaOptions and `limit` is not specified
-            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
-            warnings.warn(
-                f"fill_method={fill_method} in {type(self).__name__}.pct_change is "
-                "deprecated and will be removed in a future version. Use "
-                f"obj.{fill_type}().pct_change(fill_method=None) instead.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-            limit = None
-        elif fill_method is None and limit is not lib.no_default:
-            # `fill_method` is None but `limit` is specified
-            warnings.warn(
-                f"The limit keyword in {type(self).__name__}.pct_change is deprecated "
-                "and will be removed in a future version. It does not have any effect "
-                "when using with fill_method=None. Remove it to silence this warning.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-        elif fill_method is None:
-            # `fill_method` is None and `limit` is not specified: this should not be
-            # deprecated. TODO(3.x) allow only `fill_method=None` and raise on anything
-            # else; deprecate the `fill_method` keyword entirely. TODO(4.x) remove the
-            # `fill_method` keyword.
-            # https://github.com/pandas-dev/pandas/issues/53491#issuecomment-1728443050
-            limit = None
-        elif limit is not lib.no_default:
-            # `fill_method` is not specified but `limit` is specified
-            warnings.warn(
-                "The default fill_method='pad' and the limit keyword in "
-                f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Use obj.ffill(limit={limit}).pct_change"
-                "(fill_method=None) to retain the current behavior and silence this "
-                "warning.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
+        # GH#53491
+        msg = (
+            "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."
+        )
+        if fill_method not in (lib.no_default, None) or limit is not lib.no_default:
+            warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
+        if fill_method is lib.no_default:
+            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(msg, FutureWarning, stacklevel=find_stack_level())
+                    break
             fill_method = "pad"
-        else:
-            # `fill_method` and `limit` are both not specified
-            # GH#54981: avoid unnecessary FutureWarning
-            warnings.warn(
-                "The default fill_method='pad' and the limit keyword in "
-                f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Use obj.ffill().pct_change(fill_method=None) "
-                "to retain the current behavior and silence this warning.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-            fill_method, limit = "pad", None
+        if limit is lib.no_default:
+            limit = None
 
         axis = self._get_axis_number(kwargs.pop("axis", "index"))
         if fill_method is None:
diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py
index 579d2b1f3cd92..7d200e75b281b 100644
--- a/pandas/core/groupby/groupby.py
+++ b/pandas/core/groupby/groupby.py
@@ -5337,70 +5337,24 @@ def pct_change(
          catfish    NaN    NaN
         goldfish    0.2  0.125
         """
-        # GH#53491: deprecate the `fill_method` and `limit` keyword, except
-        # `fill_method=None` that does not fill missing values
-        if fill_method not in (lib.no_default, None) and limit is not lib.no_default:
-            # `fill_method` in FillnaOptions and `limit` is specified
-            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
-            warnings.warn(
-                f"fill_method={fill_method} and the limit keyword in "
-                f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Use obj.{fill_type}(limit={limit}).pct_change"
-                "(fill_method=None) instead.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-        elif fill_method not in (lib.no_default, None):
-            # `fill_method` in FillnaOptions and `limit` is not specified
-            fill_type = "bfill" if fill_method in ("backfill", "bfill") else "ffill"
-            warnings.warn(
-                f"fill_method={fill_method} in {type(self).__name__}.pct_change is "
-                "deprecated and will be removed in a future version. Use "
-                f"obj.{fill_type}().pct_change(fill_method=None) instead.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-            limit = None
-        elif fill_method is None and limit is not lib.no_default:
-            # `fill_method` is None but `limit` is specified
-            warnings.warn(
-                f"The limit keyword in {type(self).__name__}.pct_change is deprecated "
-                "and will be removed in a future version. It does not have any effect "
-                "when using with fill_method=None. Remove it to silence this warning.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-        elif fill_method is None:
-            # `fill_method` is None and `limit` is not specified: this should not be
-            # deprecated. TODO(3.x) allow only `fill_method=None` and raise on anything
-            # else; deprecate the `fill_method` keyword entirely. TODO(4.x) remove the
-            # `fill_method` keyword.
-            # https://github.com/pandas-dev/pandas/issues/53491#issuecomment-1728443050
+        # GH#53491
+        msg = (
+            "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."
+        )
+        if fill_method not in (lib.no_default, None) or limit is not lib.no_default:
+            warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
+        if fill_method is lib.no_default:
+            if limit is lib.no_default and any(
+                grp.isna().values.any() for _, grp in self
+            ):
+                warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
+            fill_method = "ffill"
+        if limit is lib.no_default:
             limit = None
-        elif limit is not lib.no_default:
-            # `fill_method` is not specified but `limit` is specified
-            warnings.warn(
-                "The default fill_method='pad' and the limit keyword in "
-                f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Use obj.ffill(limit={limit}).pct_change"
-                "(fill_method=None) to retain the current behavior and silence this "
-                "warning.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-            fill_method = "pad"
-        else:
-            # `fill_method` and `limit` are both not specified
-            # GH#54981: avoid unnecessary FutureWarning
-            warnings.warn(
-                "The default fill_method='pad' and the limit keyword in "
-                f"{type(self).__name__}.pct_change are deprecated and will be removed "
-                f"in a future version. Use obj.ffill().pct_change(fill_method=None) "
-                "to retain the current behavior and silence this warning.",
-                FutureWarning,
-                stacklevel=find_stack_level(),
-            )
-            fill_method, limit = "pad", None
 
         if axis is not lib.no_default:
             axis = self.obj._get_axis_number(axis)

From 6e457ef7e95446604f72e73cdfec9e271ad2ece4 Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Mon, 16 Oct 2023 00:31:16 +0800
Subject: [PATCH 5/8] update tests correspondingly

---
 pandas/core/generic.py                        | 29 ++++++++++++-------
 pandas/core/groupby/groupby.py                | 27 +++++++++++------
 pandas/tests/frame/methods/test_pct_change.py | 26 +++++++++--------
 .../tests/groupby/transform/test_transform.py |  2 +-
 .../tests/series/methods/test_pct_change.py   | 19 +++++++-----
 5 files changed, 64 insertions(+), 39 deletions(-)

diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index 75d24c68a9f85..3b066dc4bf961 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -11819,15 +11819,16 @@ def pct_change(
         APPL -0.252395 -0.011860   NaN
         """
         # GH#53491
-        msg = (
-            "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."
-        )
         if fill_method not in (lib.no_default, None) or limit is not lib.no_default:
-            warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
+            warnings.warn(
+                "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 limit is lib.no_default:
                 cols = self.items() if self.ndim == 2 else [(None, self)]
@@ -11835,8 +11836,16 @@ def pct_change(
                     mask = col.isna().values
                     mask = mask[np.argmax(~mask) :]
                     if mask.any():
-                        warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
-                    break
+                        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
diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py
index 7d200e75b281b..5c5acc7451b82 100644
--- a/pandas/core/groupby/groupby.py
+++ b/pandas/core/groupby/groupby.py
@@ -5338,20 +5338,29 @@ def pct_change(
         goldfish    0.2  0.125
         """
         # GH#53491
-        msg = (
-            "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."
-        )
         if fill_method not in (lib.no_default, None) or limit is not lib.no_default:
-            warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
+            warnings.warn(
+                "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 limit is lib.no_default and any(
                 grp.isna().values.any() for _, grp in self
             ):
-                warnings.warn(msg, FutureWarning, stacklevel=find_stack_level())
+                warnings.warn(
+                    "The default fill_method='ffill' 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(),
+                )
             fill_method = "ffill"
         if limit is lib.no_default:
             limit = None
diff --git a/pandas/tests/frame/methods/test_pct_change.py b/pandas/tests/frame/methods/test_pct_change.py
index ede212ae18ae9..c47b78b2ee7de 100644
--- a/pandas/tests/frame/methods/test_pct_change.py
+++ b/pandas/tests/frame/methods/test_pct_change.py
@@ -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):
@@ -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"
         )
 
@@ -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)
@@ -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"
         )
 
@@ -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)
 
     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
index 4a493ef3fd52c..79121897698d1 100644
--- a/pandas/tests/groupby/transform/test_transform.py
+++ b/pandas/tests/groupby/transform/test_transform.py
@@ -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):
diff --git a/pandas/tests/series/methods/test_pct_change.py b/pandas/tests/series/methods/test_pct_change.py
index 6740b8756853e..7871d785b3942 100644
--- a/pandas/tests/series/methods/test_pct_change.py
+++ b/pandas/tests/series/methods/test_pct_change.py
@@ -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)
@@ -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"
         )
 
@@ -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()
 
     expected = Series([np.nan, np.nan, 1.0, 0.5, 2.0, 1.0], index=["a", "b"] * 3)
     tm.assert_series_equal(result, expected)

From b18801ea583c9c4270cabe216eca1005ae01cefe Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Mon, 23 Oct 2023 18:31:05 +0800
Subject: [PATCH 6/8] changelog added; suggestions from Richard

---
 doc/source/whatsnew/v2.2.0.rst                 |  2 +-
 pandas/tests/frame/methods/test_pct_change.py  | 16 +++++++---------
 pandas/tests/series/methods/test_pct_change.py | 16 +++++++---------
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/doc/source/whatsnew/v2.2.0.rst b/doc/source/whatsnew/v2.2.0.rst
index 9eb5bbc8f07d5..3eedebfb9a6ee 100644
--- a/doc/source/whatsnew/v2.2.0.rst
+++ b/doc/source/whatsnew/v2.2.0.rst
@@ -269,7 +269,7 @@ Other Deprecations
 - Deprecated the option ``mode.data_manager`` and the ``ArrayManager``; only the ``BlockManager`` will be available in future versions (:issue:`55043`)
 - Deprecated the previous implementation of :class:`DataFrame.stack`; specify ``future_stack=True`` to adopt the future version (:issue:`53515`)
 - Deprecating downcasting the results of :meth:`DataFrame.fillna`, :meth:`Series.fillna`, :meth:`DataFrame.ffill`, :meth:`Series.ffill`, :meth:`DataFrame.bfill`, :meth:`Series.bfill` in object-dtype cases. To opt in to the future version, use ``pd.set_option("future.no_silent_downcasting", True)`` (:issue:`54261`)
--
+- Reverted deprecation of ``fill_method=None`` in :meth:`DataFrame.pct_change`, :meth:`Series.pct_change`, :meth:`DataFrameGroupBy.pct_change`, and :meth:`SeriesGroupBy.pct_change`. Now the keyword ``fill_method`` not being ``None`` and the limit keyword are deprecated. (:issue:`53491`)
 
 .. ---------------------------------------------------------------------------
 .. _whatsnew_220.performance:
diff --git a/pandas/tests/frame/methods/test_pct_change.py b/pandas/tests/frame/methods/test_pct_change.py
index c47b78b2ee7de..92b66e12d4356 100644
--- a/pandas/tests/frame/methods/test_pct_change.py
+++ b/pandas/tests/frame/methods/test_pct_change.py
@@ -143,15 +143,13 @@ 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
     )
 
-    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)
+    warn = None if fill_method is None else FutureWarning
+    msg = (
+        "The 'fill_method' keyword being not None and the 'limit' keyword in "
+        "DataFrame.pct_change are deprecated"
+    )
+    with tm.assert_produces_warning(warn, 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/series/methods/test_pct_change.py b/pandas/tests/series/methods/test_pct_change.py
index 7871d785b3942..9727ef3d5c27c 100644
--- a/pandas/tests/series/methods/test_pct_change.py
+++ b/pandas/tests/series/methods/test_pct_change.py
@@ -100,15 +100,13 @@ def test_pct_change_with_duplicated_indices(fill_method):
     # GH30463
     s = Series([np.nan, 1, 2, 3, 9, 18], index=["a", "b"] * 3)
 
-    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()
+    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)
     tm.assert_series_equal(result, expected)

From 33f3053b1a9ad20fb442cd86afbdc8289fc8f2eb Mon Sep 17 00:00:00 2001
From: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Date: Thu, 26 Oct 2023 01:12:09 +0800
Subject: [PATCH 7/8] updated changelog

---
 doc/source/whatsnew/v2.1.2.rst | 8 ++++++++
 doc/source/whatsnew/v2.2.0.rst | 1 -
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/doc/source/whatsnew/v2.1.2.rst b/doc/source/whatsnew/v2.1.2.rst
index 145c364728b40..952a3c770cbd7 100644
--- a/doc/source/whatsnew/v2.1.2.rst
+++ b/doc/source/whatsnew/v2.1.2.rst
@@ -8,6 +8,14 @@ including other versions of pandas.
 
 {{ header }}
 
+.. ---------------------------------------------------------------------------
+.. _whatsnew_220.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:
 
diff --git a/doc/source/whatsnew/v2.2.0.rst b/doc/source/whatsnew/v2.2.0.rst
index 554d3caceb18b..1b8f8f90846c3 100644
--- a/doc/source/whatsnew/v2.2.0.rst
+++ b/doc/source/whatsnew/v2.2.0.rst
@@ -288,7 +288,6 @@ Other Deprecations
 - Deprecated the option ``mode.data_manager`` and the ``ArrayManager``; only the ``BlockManager`` will be available in future versions (:issue:`55043`)
 - Deprecated the previous implementation of :class:`DataFrame.stack`; specify ``future_stack=True`` to adopt the future version (:issue:`53515`)
 - Deprecating downcasting the results of :meth:`DataFrame.fillna`, :meth:`Series.fillna`, :meth:`DataFrame.ffill`, :meth:`Series.ffill`, :meth:`DataFrame.bfill`, :meth:`Series.bfill` in object-dtype cases. To opt in to the future version, use ``pd.set_option("future.no_silent_downcasting", True)`` (:issue:`54261`)
-- Reverted deprecation of ``fill_method=None`` in :meth:`DataFrame.pct_change`, :meth:`Series.pct_change`, :meth:`DataFrameGroupBy.pct_change`, and :meth:`SeriesGroupBy.pct_change`. Now the keyword ``fill_method`` not being ``None`` and the limit keyword are deprecated. (:issue:`53491`)
 
 .. ---------------------------------------------------------------------------
 .. _whatsnew_220.performance:

From dd007551e1583b8f7154f684983c89f38071547a Mon Sep 17 00:00:00 2001
From: Richard Shadrach <45562402+rhshadrach@users.noreply.github.com>
Date: Wed, 25 Oct 2023 20:55:12 -0400
Subject: [PATCH 8/8] Update v2.1.2.rst

---
 doc/source/whatsnew/v2.1.2.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/source/whatsnew/v2.1.2.rst b/doc/source/whatsnew/v2.1.2.rst
index 952a3c770cbd7..37a07a9e20b1d 100644
--- a/doc/source/whatsnew/v2.1.2.rst
+++ b/doc/source/whatsnew/v2.1.2.rst
@@ -9,7 +9,7 @@ including other versions of pandas.
 {{ header }}
 
 .. ---------------------------------------------------------------------------
-.. _whatsnew_220.deprecations:
+.. _whatsnew_212.deprecations:
 
 Deprecations
 ~~~~~~~~~~~~