Series pct_change fill_method behavior

doc/source/whatsnew/v0.25.0.rst

@@ -246,6 +246,7 @@ Other API Changes
 - :class:`DatetimeTZDtype` will now standardize pytz timezones to a common timezone instance (:issue:`24713`)
 - :class:`Timestamp` and :class:`Timedelta` scalars now implement the :meth:`to_numpy` method as aliases to :meth:`Timestamp.to_datetime64` and :meth:`Timedelta.to_timedelta64`, respectively. (:issue:`24653`)
 - :meth:`Timestamp.strptime` will now rise a ``NotImplementedError`` (:issue:`25016`)
+- Default `skipna=True` for :meth:`Series.pct_change` and :meth:`DataFrame.pct_change` will drop NAs before calculation (:issue:`25006`)
 - Comparing :class:`Timestamp` with unsupported objects now returns :py:obj:`NotImplemented` instead of raising ``TypeError``. This implies that unsupported rich comparisons are delegated to the other object, and are now consistent with Python 3 behavior for ``datetime`` objects (:issue:`24011`)
 - Bug in :meth:`DatetimeIndex.snap` which didn't preserving the ``name`` of the input :class:`Index` (:issue:`25575`)
 - The ``arg`` argument in :meth:`pandas.core.groupby.DataFrameGroupBy.agg` has been renamed to ``func`` (:issue:`26089`)

pandas/core/generic.py

@@ -9993,6 +9993,10 @@ def _check_percentile(self, q):
             The number of consecutive NAs to fill before stopping.
         freq : DateOffset, timedelta, or offset alias string, optional
             Increment to use from time series API (e.g. 'M' or BDay()).
+        skipna : bool, default True
+            Exclude NA/null values before computing percent change.
+
+            .. versionadded:: 0.25.0
         **kwargs
             Additional keyword arguments are passed into
             `DataFrame.shift` or `Series.shift`.
@@ -10009,6 +10013,11 @@ def _check_percentile(self, q):
         Series.shift : Shift the index by some number of periods.
         DataFrame.shift : Shift the index by some number of periods.
 
+        Notes
+        -----
+        The default `skipna=True` drops NAs before computing the percentage
+        change, and the results are reindexed like the original calling object.
+
         Examples
         --------
         **Series**
@@ -10032,22 +10041,42 @@ def _check_percentile(self, q):
         2   -0.055556
         dtype: float64
 
-        See the percentage change in a Series where filling NAs with last
-        valid observation forward to next valid.
+        See how the computing of percentage change is performed in a Series
+        with NAs. With default `skipna=True`, NAs are dropped before the
+        computation and eventually the results are reindexed like the original
+        object, thus keeping the original NAs.
 
-        >>> s = pd.Series([90, 91, None, 85])
+        >>> s = pd.Series([90, 91, np.nan, 85, np.nan, 95])
         >>> s
         0    90.0
         1    91.0
         2     NaN
         3    85.0
+        4     NaN
+        5    95.0
+        dtype: float64
+
+        >>> s.pct_change()
+        0         NaN
+        1    0.011111
+        2         NaN
+        3   -0.065934
+        4         NaN
+        5    0.117647
         dtype: float64
 
+        On the other hand, if a fill method is passed, NAs are filled before
+        the computation. For example, before the computation of percentage
+        change, forward fill method `ffill` first fills NAs with last valid
+        observation forward to next valid.
+
         >>> s.pct_change(fill_method='ffill')
         0         NaN
         1    0.011111
         2    0.000000
         3   -0.065934
+        4    0.000000
+        5    0.117647
         dtype: float64
 
         **DataFrame**
@@ -10089,14 +10118,75 @@ def _check_percentile(self, q):
               2016      2015      2014
         GOOG   NaN -0.151997 -0.086016
         APPL   NaN  0.337604  0.012002
+
+        In a DataFrame with NAs, when computing the percentage change with
+        default `skipna=True`, NAs are first droppped on each column/row, and
+        the results are eventually reindexed as originally.
+
+        >>> df = pd.DataFrame({
+        ...     'a': [90, 91, np.nan, 85, np.nan, 95],
+        ...     'b': [91, np.nan, 85, np.nan, 95, np.nan],
+        ...     'c': [np.nan, 85, np.nan, 95, np.nan, np.nan]})
+        >>> df
+              a     b     c
+        0  90.0  91.0   NaN
+        1  91.0   NaN  85.0
+        2   NaN  85.0   NaN
+        3  85.0   NaN  95.0
+        4   NaN  95.0   NaN
+        5  95.0   NaN   NaN
+
+        >>> df.pct_change()
+                  a         b         c
+        0       NaN       NaN       NaN
+        1  0.011111       NaN       NaN
+        2       NaN -0.065934       NaN
+        3 -0.065934       NaN  0.117647
+        4       NaN  0.117647       NaN
+        5  0.117647       NaN       NaN
+
+        >>> df.pct_change(axis=1)
+            a         b         c
+        0 NaN  0.011111       NaN
+        1 NaN       NaN -0.065934
+        2 NaN       NaN       NaN
+        3 NaN       NaN  0.117647
+        4 NaN       NaN       NaN
+        5 NaN       NaN       NaN
+
+        Otherwise, if a fill method is passed, NAs are filled before the
+        computation.
+
+        >>> df.pct_change(fill_method='ffill')
+                  a         b         c
+        0       NaN       NaN       NaN
+        1  0.011111  0.000000       NaN
+        2  0.000000 -0.065934  0.000000
+        3 -0.065934  0.000000  0.117647
+        4  0.000000  0.117647  0.000000
+        5  0.117647  0.000000  0.000000
         """
 
     @Appender(_shared_docs['pct_change'] % _shared_doc_kwargs)
-    def pct_change(self, periods=1, fill_method='pad', limit=None, freq=None,
-                   **kwargs):
-        # TODO: Not sure if above is correct - need someone to confirm.
+    def pct_change(self, periods=1, fill_method=None, limit=None, freq=None,
+                   skipna=None, **kwargs):
+        if fill_method is not None and skipna:
+            raise ValueError("cannot pass both fill_method and skipna")
+        elif limit is not None and skipna:
+            raise ValueError("cannot pass both limit and skipna")
+        if fill_method is None and limit is None and skipna is None:
+            skipna = True
         axis = self._get_axis_number(kwargs.pop('axis', self._stat_axis_name))
-        if fill_method is None:
+        if skipna and isinstance(self, ABCDataFrame):
+            # If DataFrame, apply to each column/row
+            return self.apply(
+                lambda s: s.pct_change(periods=periods, freq=freq,
+                                       skipna=skipna, **kwargs),
+                axis=axis
+            )
+        if skipna:
+            data = self.dropna()
+        elif fill_method is None:
             data = self
         else:
             data = self.fillna(method=fill_method, limit=limit, axis=axis)
@@ -10107,6 +10197,8 @@ def pct_change(self, periods=1, fill_method='pad', limit=None, freq=None,
         if freq is None:
             mask = isna(com.values_from_object(data))
             np.putmask(rs.values, mask, np.nan)
+        if skipna:
+            rs = rs.reindex_like(self)
         return rs
 
     def _agg_by_level(self, name, axis=0, level=0, skipna=True, **kwargs):

pandas/tests/frame/test_analytics.py

@@ -1364,6 +1364,26 @@ def test_pct_change(self):
 
             tm.assert_frame_equal(result, expected)
 
+    @pytest.mark.parametrize("skipna, periods, expected_vals", [
+        (True, 1, [[np.nan, np.nan], [np.nan, np.nan], [1., np.nan], [0.5, 1.],
+                   [np.nan, 0.5], [0.33333333, np.nan], [np.nan, 0.33333333]]),
+        (True, 2, [[np.nan, np.nan], [np.nan, np.nan], [np.nan, np.nan],
+                   [2., np.nan], [np.nan, 2.], [1., np.nan], [np.nan, 1.]]),
+        (False, 1, [[np.nan, np.nan], [np.nan, np.nan], [1., np.nan],
+                    [0.5, 1.], [np.nan, 0.5], [np.nan, np.nan],
+                    [np.nan, np.nan]]),
+        (False, 2, [[np.nan, np.nan], [np.nan, np.nan], [np.nan, np.nan],
+                    [2., np.nan], [np.nan, 2.], [0.33333333, np.nan],
+                    [np.nan, 0.33333333]])
+    ])
+    def test_pct_change_skipna(self, skipna, periods, expected_vals):
+        # GH25006
+        df = DataFrame([[np.nan, np.nan], [1., np.nan], [2., 1.], [3., 2.],
+                        [np.nan, 3.], [4., np.nan], [np.nan, 4.]])
+        result = df.pct_change(skipna=skipna, periods=periods)
+        expected = DataFrame(expected_vals)
+        tm.assert_frame_equal(result, expected)
+
     # ----------------------------------------------------------------------
     # Index of max / min
 

pandas/tests/frame/test_timeseries.py

@@ -116,10 +116,10 @@ def test_diff_axis(self):
             [[np.nan, np.nan], [2., 2.]]))
 
     def test_pct_change(self):
-        rs = self.tsframe.pct_change(fill_method=None)
+        rs = self.tsframe.pct_change(skipna=False, fill_method=None)
         assert_frame_equal(rs, self.tsframe / self.tsframe.shift(1) - 1)
 
-        rs = self.tsframe.pct_change(2)
+        rs = self.tsframe.pct_change(periods=2)
         filled = self.tsframe.fillna(method='pad')
         assert_frame_equal(rs, filled / filled.shift(2) - 1)
 
@@ -138,7 +138,7 @@ def test_pct_change_shift_over_nas(self):
 
         df = DataFrame({'a': s, 'b': s})
 
-        chg = df.pct_change()
+        chg = df.pct_change(fill_method='ffill')
         expected = Series([np.nan, 0.5, 0., 2.5 / 1.5 - 1, .2])
         edf = DataFrame({'a': expected, 'b': expected})
         assert_frame_equal(chg, edf)
@@ -155,7 +155,7 @@ def test_pct_change_periods_freq(self, freq, periods, fill_method, limit):
         rs_freq = self.tsframe.pct_change(freq=freq,
                                           fill_method=fill_method,
                                           limit=limit)
-        rs_periods = self.tsframe.pct_change(periods,
+        rs_periods = self.tsframe.pct_change(periods=periods,
                                              fill_method=fill_method,
                                              limit=limit)
         assert_frame_equal(rs_freq, rs_periods)
@@ -165,7 +165,7 @@ def test_pct_change_periods_freq(self, freq, periods, fill_method, limit):
         rs_freq = empty_ts.pct_change(freq=freq,
                                       fill_method=fill_method,
                                       limit=limit)
-        rs_periods = empty_ts.pct_change(periods,
+        rs_periods = empty_ts.pct_change(periods=periods,
                                          fill_method=fill_method,
                                          limit=limit)
         assert_frame_equal(rs_freq, rs_periods)

pandas/tests/generic/test_generic.py

@@ -576,6 +576,25 @@ def test_pct_change(self, periods, fill_method, limit, exp):
         else:
             tm.assert_series_equal(res, Series(exp))
 
+    @pytest.mark.parametrize('fill_method, limit', [
+        ('backfill', None),
+        ('bfill', None),
+        ('pad', None),
+        ('ffill', None),
+        (None, 1)
+    ])
+    def test_pct_change_skipna_raises(self, fill_method, limit):
+        # GH25006
+        vals = [np.nan, np.nan, 1, 2, np.nan, 4, 10, np.nan]
+        obj = self._typ(vals)
+        if fill_method:
+            msg = "cannot pass both fill_method and skipna"
+        else:
+            msg = "cannot pass both limit and skipna"
+        with pytest.raises(ValueError, match=msg):
+            obj.pct_change(skipna=True, fill_method=fill_method,
+                           limit=limit)
+
 
 class TestNDFrame:
     # tests that don't fit elsewhere

pandas/tests/series/test_analytics.py

@@ -2,7 +2,6 @@
 import operator
 
 import numpy as np
-from numpy import nan
 import pytest
 
 import pandas.util._test_decorators as td
@@ -206,6 +205,18 @@ def test_cummax_timedelta64(self):
         result = s.cummax(skipna=False)
         tm.assert_series_equal(expected, result)
 
+    @pytest.mark.parametrize("periods, expected_vals", [
+        (1, [np.nan, np.nan, 1.0, 0.5, np.nan, 0.333333333333333, np.nan]),
+        (2, [np.nan, np.nan, np.nan, 2.0, np.nan, 1.0, np.nan])
+    ])
+    def test_pct_change_skipna(self, periods, expected_vals):
+        # GH25006
+        vals = [np.nan, 1., 2., 3., np.nan, 4., np.nan]
+        s = Series(vals)
+        result = s.pct_change(skipna=True, periods=periods)
+        expected = Series(expected_vals)
+        assert_series_equal(expected, result)
+
     def test_npdiff(self):
         pytest.skip("skipping due to Series no longer being an "
                     "ndarray")
@@ -214,7 +225,7 @@ def test_npdiff(self):
         s = Series(np.arange(5))
 
         r = np.diff(s)
-        assert_series_equal(Series([nan, 0, 0, 0, nan]), r)
+        assert_series_equal(Series([np.nan, 0, 0, 0, np.nan]), r)
 
     def _check_accum_op(self, name, datetime_series_, check_dtype=True):
         func = getattr(np, name)
@@ -437,14 +448,14 @@ def test_count(self, datetime_series):
 
         assert datetime_series.count() == np.isfinite(datetime_series).sum()
 
-        mi = MultiIndex.from_arrays([list('aabbcc'), [1, 2, 2, nan, 1, 2]])
+        mi = MultiIndex.from_arrays([list('aabbcc'), [1, 2, 2, np.nan, 1, 2]])
         ts = Series(np.arange(len(mi)), index=mi)
 
         left = ts.count(level=1)
-        right = Series([2, 3, 1], index=[1, 2, nan])
+        right = Series([2, 3, 1], index=[1, 2, np.nan])
         assert_series_equal(left, right)
 
-        ts.iloc[[0, 3, 5]] = nan
+        ts.iloc[[0, 3, 5]] = np.nan
         assert_series_equal(ts.count(level=1), right - 1)
 
     def test_dot(self):
@@ -663,11 +674,11 @@ def test_cummethods_bool(self):
             result = getattr(s, method)()
             assert_series_equal(result, expected)
 
-        e = pd.Series([False, True, nan, False])
-        cse = pd.Series([0, 1, nan, 1], dtype=object)
-        cpe = pd.Series([False, 0, nan, 0])
-        cmin = pd.Series([False, False, nan, False])
-        cmax = pd.Series([False, True, nan, True])
+        e = pd.Series([False, True, np.nan, False])
+        cse = pd.Series([0, 1, np.nan, 1], dtype=object)
+        cpe = pd.Series([False, 0, np.nan, 0])
+        cmin = pd.Series([False, False, np.nan, False])
+        cmax = pd.Series([False, True, np.nan, True])
         expecteds = {'cumsum': cse,
                      'cumprod': cpe,
                      'cummin': cmin,
@@ -945,15 +956,13 @@ def test_shift_categorical(self):
         assert_index_equal(s.values.categories, sn2.values.categories)
 
     def test_unstack(self):
-        from numpy import nan
-
         index = MultiIndex(levels=[['bar', 'foo'], ['one', 'three', 'two']],
                            codes=[[1, 1, 0, 0], [0, 1, 0, 2]])
 
         s = Series(np.arange(4.), index=index)
         unstacked = s.unstack()
 
-        expected = DataFrame([[2., nan, 3.], [0., 1., nan]],
+        expected = DataFrame([[2., np.nan, 3.], [0., 1., np.nan]],
                              index=['bar', 'foo'],
                              columns=['one', 'three', 'two'])
 
@@ -977,17 +986,17 @@ def test_unstack(self):
         idx = pd.MultiIndex.from_arrays([[101, 102], [3.5, np.nan]])
         ts = pd.Series([1, 2], index=idx)
         left = ts.unstack()
-        right = DataFrame([[nan, 1], [2, nan]], index=[101, 102],
-                          columns=[nan, 3.5])
+        right = DataFrame([[np.nan, 1], [2, np.nan]], index=[101, 102],
+                          columns=[np.nan, 3.5])
         assert_frame_equal(left, right)
 
         idx = pd.MultiIndex.from_arrays([['cat', 'cat', 'cat', 'dog', 'dog'
                                           ], ['a', 'a', 'b', 'a', 'b'],
                                          [1, 2, 1, 1, np.nan]])
         ts = pd.Series([1.0, 1.1, 1.2, 1.3, 1.4], index=idx)
-        right = DataFrame([[1.0, 1.3], [1.1, nan], [nan, 1.4], [1.2, nan]],
-                          columns=['cat', 'dog'])
-        tpls = [('a', 1), ('a', 2), ('b', nan), ('b', 1)]
+        right = DataFrame([[1.0, 1.3], [1.1, np.nan], [np.nan, 1.4],
+                           [1.2, np.nan]], columns=['cat', 'dog'])
+        tpls = [('a', 1), ('a', 2), ('b', np.nan), ('b', 1)]
         right.index = pd.MultiIndex.from_tuples(tpls)
         assert_frame_equal(ts.unstack(level=0), right)