Repository: pandas-dev/pandas
Pull request: BUG: pct_change wrong result when there are duplicated indices
- [x] closes  #30463
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
pct_change can't work well with groupby, when fill_method =None
#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd
import numpy as np
data = pd.DataFrame(np.random.random((10,2)), index=['a', 'b']*5)
data.iloc[1:3,:] = np.nan
```
for pct_change function, when the fill_method = None, it works

```
data.pct_change(1, fill_method=None, limit=1)
	0	1
a	NaN	NaN
b	NaN	NaN
a	NaN	NaN
b	NaN	NaN
a	-0.498169	-0.568501
b	-0.315982	1.340587
a	1.341901	-0.489576
b	0.088594	-0.691063
a	-0.514451	0.054695
b	0.844514	-0.604511
```
but when use it with gourpby , it raise a error

```
data.reset_index().groupby('index')[0].pct_change(1, fill_method=None, limit=1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-18-60898304743e> in <module>
----> 1 data.reset_index().groupby('index')[0].pct_change(1, fill_method=None, limit=1)

~/opt/anaconda3/lib/python3.7/site-packages/pandas/core/groupby/generic.py in pct_change(self, periods, fill_method, limit, freq)
   1344                 )
   1345             )
-> 1346         filled = getattr(self, fill_method)(limit=limit)
   1347         fill_grp = filled.groupby(self.grouper.labels)
   1348         shifted = fill_grp.shift(periods=periods, freq=freq)

TypeError: getattr(): attribute name must be string
```
else, `limit = 0` worked with gourpby, but can't run in pct_change

```
data.reset_index().groupby('index')[0].pct_change(1, fill_method='pad', limit=0)  
0         NaN
1         NaN
2         NaN
3         NaN
4         NaN
5   -0.656739
6    0.601904
7    1.549380
8   -0.471434
9   -0.104398
Name: 0, dtype: float64  
``` 
```
data[0].pct_change(1, fill_method='pad', limit=0)  

~/opt/anaconda3/lib/python3.7/site-packages/pandas/core/missing.py in pad_2d(values, limit, mask, dtype)
    546 
    547     if np.all(values.shape):
--> 548         algos.pad_2d_inplace(values, mask, limit=limit)
    549     else:
    550         # for test coverage

pandas/_libs/algos.pyx in pandas._libs.algos.pad_2d_inplace()

ValueError: Limit must be greater than 0
```

pandas version is 0.25.1

Hints from issue discussion:
pls try on master 
Tried on master. It does seem we have an issue here.

```python
np.random.seed(0)
data = pd.DataFrame(np.random.random((6, 2)), index=['a', 'b'] * 3)
data.iloc[0, :] = np.nan
data

          0         1
a       NaN       NaN
b  0.602763  0.544883
a  0.423655  0.645894
b  0.437587  0.891773
a  0.963663  0.383442
b  0.791725  0.528895
```

This gives us all null values 
```python
data.pct_change(fill_method=None)

    0   1
a NaN NaN
b NaN NaN
a NaN NaN
b NaN NaN
a NaN NaN
b NaN NaN
```

GroupBy.pct_change with default parameters seems ok
```python
data.groupby(level=0).pct_change()

          0         1
a       NaN       NaN
b       NaN       NaN
a       NaN       NaN
b -0.274032  0.636632
a  1.274641 -0.406340
b  0.809297 -0.406918
```

However, this raises an error
```python
data.groupby(level=0).pct_change(fill_method=None)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\fujia\PycharmProjects\pandas\pandas\core\groupby\groupby.py", line 2365, in pct_change
    filled = getattr(self, fill_method)(limit=limit)
TypeError: getattr(): attribute name must be string
```

This is even more weird. Somehow only index=b has values and they are all the same values.
```python
data = pd.DataFrame(np.random.random((6, 2)), index=['a', 'b'] * 3)
data.iloc[2, :] = np.nan
data

          0         1
a  0.568045  0.925597
b  0.071036  0.087129
a       NaN       NaN
b  0.778157  0.870012
a  0.978618  0.799159
b  0.461479  0.780529

data.pct_change(fill_method=None)
          0         1
a       NaN       NaN
b -0.874946 -0.905867
a       NaN       NaN
b -0.874946 -0.905867
a       NaN       NaN
b -0.874946 -0.905867
```
I will take a look at this
take
There are two separate issues:
1. `Series / DataFrame.pct_change` incorrectly reindex(es) results when `freq is None`
1. `SeriesGroupBY / DataFrameGroupBY` did not handle the case when `fill_method is None`

Will create separate PRs to address them

Commit message: Merge branch 'master' into fix_pct_change_duplicated_indices

Patch to review:
diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst
index a6ba7770dadcc..7b0c4a977a919 100755
--- a/doc/source/whatsnew/v1.0.0.rst
+++ b/doc/source/whatsnew/v1.0.0.rst
@@ -765,6 +765,7 @@ Numeric
 - Bug in :class:`NumericIndex` construction that caused :class:`UInt64Index` to be casted to :class:`Float64Index` when integers in the ``np.uint64`` range were used to index a :class:`DataFrame` (:issue:`28279`)
 - Bug in :meth:`Series.interpolate` when using method=`index` with an unsorted index, would previously return incorrect results. (:issue:`21037`)
 - Bug in :meth:`DataFrame.round` where a :class:`DataFrame` with a :class:`CategoricalIndex` of :class:`IntervalIndex` columns would incorrectly raise a ``TypeError`` (:issue:`30063`)
+- Bug in :meth:`NDFrame.pct_change` when there are duplicated indices (:issue:`30463`)
 - Bug in :class:`DataFrame` cumulative operations (e.g. cumsum, cummax) incorrect casting to object-dtype (:issue:`19296`)
 
 Conversion
diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index b896721469f1f..55dcc81139276 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -9901,11 +9901,11 @@ def pct_change(self, periods=1, fill_method="pad", limit=None, freq=None, **kwar
             data = self.fillna(method=fill_method, limit=limit, axis=axis)
 
         rs = data.div(data.shift(periods=periods, freq=freq, axis=axis, **kwargs)) - 1
-        rs = rs.loc[~rs.index.duplicated()]
-        rs = rs.reindex_like(data)
-        if freq is None:
-            mask = isna(com.values_from_object(data))
-            np.putmask(rs.values, mask, np.nan)
+        if freq is not None:
+            # Shift method is implemented differently when freq is not None
+            # We want to restore the original index
+            rs = rs.loc[~rs.index.duplicated()]
+            rs = rs.reindex_like(data)
         return rs
 
     def _agg_by_level(self, name, axis=0, level=0, skipna=True, **kwargs):
diff --git a/pandas/tests/frame/methods/test_pct_change.py b/pandas/tests/frame/methods/test_pct_change.py
index 0c15533c37f01..ac13a5e146043 100644
--- a/pandas/tests/frame/methods/test_pct_change.py
+++ b/pandas/tests/frame/methods/test_pct_change.py
@@ -76,3 +76,21 @@ def test_pct_change_periods_freq(
         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)
         tm.assert_frame_equal(rs_freq, rs_periods)
+
+
+@pytest.mark.parametrize("fill_method", ["pad", "ffill", None])
+def test_pct_change_with_duplicated_indices(fill_method):
+    # GH30463
+    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)
+    if fill_method is None:
+        second_column = [np.nan, np.inf, np.nan, np.nan, 2.0, 1.0]
+    else:
+        second_column = [np.nan, np.inf, 0.0, 2.0, 2.0, 1.0]
+    expected = DataFrame(
+        {0: [np.nan, np.nan, 1.0, 0.5, 2.0, 1.0], 1: second_column},
+        index=["a", "b"] * 3,
+    )
+    tm.assert_frame_equal(result, expected)
diff --git a/pandas/tests/series/methods/test_pct_change.py b/pandas/tests/series/methods/test_pct_change.py
index abc5c498813ef..aa01543132841 100644
--- a/pandas/tests/series/methods/test_pct_change.py
+++ b/pandas/tests/series/methods/test_pct_change.py
@@ -68,3 +68,12 @@ def test_pct_change_periods_freq(
         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)
         tm.assert_series_equal(rs_freq, rs_periods)
+
+
+@pytest.mark.parametrize("fill_method", ["pad", "ffill", None])
+def test_pct_change_with_duplicated_indices(fill_method):
+    # GH30463
+    s = Series([np.nan, 1, 2, 3, 9, 18], index=["a", "b"] * 3)
+    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)


Code review comments:
diff_hunk: @@ -765,6 +765,7 @@ Numeric
 - Bug in :class:`NumericIndex` construction that caused :class:`UInt64Index` to be casted to :class:`Float64Index` when integers in the ``np.uint64`` range were used to index a :class:`DataFrame` (:issue:`28279`)
 - Bug in :meth:`Series.interpolate` when using method=`index` with an unsorted index, would previously return incorrect results. (:issue:`21037`)
 - Bug in :meth:`DataFrame.round` where a :class:`DataFrame` with a :class:`CategoricalIndex` of :class:`IntervalIndex` columns would incorrectly raise a ``TypeError`` (:issue:`30063`)
+- Bug in :meth:`NDFrame.pct_change` when there are duplicated indices (:issue:`30463`)
path: doc/source/whatsnew/v1.0.0.rst
text: @user1:
I don't think this renders, instead use Series.pct_change and DataFrame.pct_change

@author:
done