pct change bug issue 21200

doc/source/whatsnew/v0.23.5.txt

@@ -42,7 +42,6 @@ Bug Fixes
 **Groupby/Resample/Rolling**
 
 - Bug in :meth:`DataFrame.resample` when resampling ``NaT`` in ``TimeDeltaIndex`` (:issue:`13223`).
--
 
 **Missing**
 
@@ -52,3 +51,33 @@ Bug Fixes
 **I/O**
 
 - Bug in :func:`read_csv` that caused it to raise ``OverflowError`` when trying to use 'inf' as ``na_value`` with integer index column (:issue:`17128`)
+
+Backwards incompatible API changes
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- Bug where calling :func:`SeriesGroupBy.pct_change` or :func:`DataFrameGroupBy.pct_change` would ignore groups when calculating the percent change (:issue:`21235`)
+
+**New behavior**:
+
+.. ipython:: python
+
+   import pandas as pd
+   df = pd.DataFrame({'grp': ['a', 'a', 'b'], 'foo': [1.0, 1.1, 2.2]})
+   df
+
+   df.groupby('grp').pct_change()
+
+
+**Previous behavior**:
+
+.. code-block:: ipython
+
+   In [1]: df.groupby('grp').pct_change()
+   Out[1]:
+      foo
+   0  NaN
+   1  0.1
+   2  1.0
+
+
+

pandas/core/groupby/generic.py

@@ -1221,9 +1221,15 @@ def _apply_to_column_groupbys(self, func):
         return func(self)
 
     def pct_change(self, periods=1, fill_method='pad', limit=None, freq=None):
-        """Calculate percent change of each value to previous entry in group"""
+        """Calcuate pct_change of each value to previous entry in group"""
+        # TODO: Remove this conditional when #23918 is fixed
+        if freq:
+            return self.apply(lambda x: x.pct_change(periods=periods,
+                                                     fill_method=fill_method,
+                                                     limit=limit, freq=freq))
         filled = getattr(self, fill_method)(limit=limit)
-        shifted = filled.shift(periods=periods, freq=freq)
+        fill_grp = filled.groupby(self.grouper.labels)
+        shifted = fill_grp.shift(periods=periods, freq=freq)
 
         return (filled / shifted) - 1
 

pandas/core/groupby/groupby.py

@@ -1997,11 +1997,10 @@ def pct_change(self, periods=1, fill_method='pad', limit=None, freq=None,
                                                      fill_method=fill_method,
                                                      limit=limit, freq=freq,
                                                      axis=axis))
-
-        filled = getattr(self, fill_method)(limit=limit).drop(
-            self.grouper.names, axis=1)
-        shifted = filled.shift(periods=periods, freq=freq)
-
+        filled = getattr(self, fill_method)(limit=limit)
+        filled = filled.drop(self.grouper.names, axis=1)
+        fill_grp = filled.groupby(self.grouper.labels)
+        shifted = fill_grp.shift(periods=periods, freq=freq)
         return (filled / shifted) - 1
 
     @Substitution(name='groupby')

pandas/tests/groupby/test_transform.py

@@ -765,36 +765,37 @@ def test_pad_stable_sorting(fill_method):
 
 
 @pytest.mark.parametrize("test_series", [True, False])
+@pytest.mark.parametrize("freq", [None, 'D'])
 @pytest.mark.parametrize("periods,fill_method,limit", [
     (1, 'ffill', None), (1, 'ffill', 1),
     (1, 'bfill', None), (1, 'bfill', 1),
     (-1, 'ffill', None), (-1, 'ffill', 1),
-    (-1, 'bfill', None), (-1, 'bfill', 1)])
-def test_pct_change(test_series, periods, fill_method, limit):
-    vals = [np.nan, np.nan, 1, 2, 4, 10, np.nan, np.nan]
-    exp_vals = Series(vals).pct_change(periods=periods,
-                                       fill_method=fill_method,
-                                       limit=limit).tolist()
-
-    df = DataFrame({'key': ['a'] * len(vals) + ['b'] * len(vals),
-                    'vals': vals * 2})
-    grp = df.groupby('key')
-
-    def get_result(grp_obj):
-        return grp_obj.pct_change(periods=periods,
-                                  fill_method=fill_method,
-                                  limit=limit)
+    (-1, 'bfill', None), (-1, 'bfill', 1),
+])
+def test_pct_change(test_series, freq, periods, fill_method, limit):
+    # GH  21200, 21621
+    if freq == 'D':
+        pytest.xfail("'freq' test not necessary until #23918 completed and"
+                     "freq is used in the vectorized approach")
+
+    vals = [3, np.nan, np.nan, np.nan, 1, 2, 4, 10, np.nan, 4]
+    keys = ['a', 'b']
+    key_v = np.repeat(keys, len(vals))
+    df = DataFrame({'key': key_v, 'vals': vals * 2})
+
+    df_g = getattr(df.groupby('key'), fill_method)(limit=limit)
+    grp = df_g.groupby('key')
+
+    expected = grp['vals'].obj / grp['vals'].shift(periods) - 1
 
     if test_series:
-        exp = pd.Series(exp_vals * 2)
-        exp.name = 'vals'
-        grp = grp['vals']
-        result = get_result(grp)
-        tm.assert_series_equal(result, exp)
+        result = df.groupby('key')['vals'].pct_change(
+            periods=periods, fill_method=fill_method, limit=limit, freq=freq)
+        tm.assert_series_equal(result, expected)
     else:
-        exp = DataFrame({'vals': exp_vals * 2})
-        result = get_result(grp)
-        tm.assert_frame_equal(result, exp)
+        result = df.groupby('key').pct_change(
+            periods=periods, fill_method=fill_method, limit=limit, freq=freq)
+        tm.assert_frame_equal(result, expected.to_frame('vals'))
 
 
 @pytest.mark.parametrize("func", [np.any, np.all])