Repository: pandas-dev/pandas
Pull request: ENH: fill_value argument for shift #15486
- [ ] closes #15486
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Problem statement:
ENH: fill_value argument for shift
#### Feature Request

At the moment, the Series/DataFrame `shift` methods always fill with nans, which causes an upcast for boolean/integer types. In cases where there is an appropriate non-nan fill value, it would be useful to provide this value in `shift`, rather than having to correctly downcast the result. This would presumably also be more efficient than `.shift().fillna()` in those cases where no upcast is required.

#### Example Code

```python
>>> pd.Series([True]).shift(fill_value=False)
0    False
dtype: bool
```

Hints from issue discussion:
sure this is reasonable to add. Also wouldn't be very difficult, mostly just need to pass this thru to the lower level functions (where a fill_values is found).

care to do a PR?
This is a copy of #15527 where work seems to be in progress?
@linebp this is the original reported issue, #15527 is a possible patch for this.
Looks like this still needs doing. I'm going to try and give this ago if nobody is working on it. 
First stab in this PR https://github.com/pandas-dev/pandas/pull/20460/files

Commit message: !U remove doc from the doc

Patch to review:
diff --git a/pandas/core/arrays/base.py b/pandas/core/arrays/base.py
index 15abffa9a5d23..dd9c08053e0ed 100644
--- a/pandas/core/arrays/base.py
+++ b/pandas/core/arrays/base.py
@@ -444,7 +444,7 @@ def dropna(self):
         """
         return self[~self.isna()]
 
-    def shift(self, periods=1):
+    def shift(self, periods=1, fill_value=None):
         # type: (int) -> ExtensionArray
         """
         Shift values by desired number.
@@ -460,6 +460,12 @@ def shift(self, periods=1):
             The number of periods to shift. Negative values are allowed
             for shifting backwards.
 
+        fill_value : object, optional
+            The scalar value to use for newly introduced missing values.
+            The default is ``self.dtype.na_value``
+
+            .. versionadded:: 0.24.0
+
         Returns
         -------
         shifted : ExtensionArray
@@ -478,8 +484,11 @@ def shift(self, periods=1):
         if not len(self) or periods == 0:
             return self.copy()
 
+        if fill_value is None:
+            fill_value = self.dtype.na_value
+
         empty = self._from_sequence(
-            [self.dtype.na_value] * min(abs(periods), len(self)),
+            [fill_value] * min(abs(periods), len(self)),
             dtype=self.dtype
         )
         if periods > 0:
diff --git a/pandas/core/arrays/categorical.py b/pandas/core/arrays/categorical.py
index 6e96fc75daec9..3377ce91bae60 100644
--- a/pandas/core/arrays/categorical.py
+++ b/pandas/core/arrays/categorical.py
@@ -1255,7 +1255,7 @@ def shape(self):
 
         return tuple([len(self._codes)])
 
-    def shift(self, periods):
+    def shift(self, periods, fill_value=None):
         """
         Shift Categorical by desired number of periods.
 
@@ -1263,6 +1263,11 @@ def shift(self, periods):
         ----------
         periods : int
             Number of periods to move, can be positive or negative
+        fill_value : object, optional
+            The scalar value to use for newly introduced missing values.
+            The default is ``-1``.
+
+            .. versionadded:: 0.24.0
 
         Returns
         -------
@@ -1275,10 +1280,18 @@ def shift(self, periods):
             raise NotImplementedError("Categorical with ndim > 1.")
         if np.prod(codes.shape) and (periods != 0):
             codes = np.roll(codes, ensure_platform_int(periods), axis=0)
+            if fill_value is None:
+                fill_value = -1
+            elif fill_value in self.categories:
+                fill_value = self.categories.get_loc(fill_value)
+            else:
+                raise ValueError("'fill_value={}' is not present "
+                                 "in this Categorical's "
+                                 "categories".format(fill_value))
             if periods > 0:
-                codes[:periods] = -1
+                codes[:periods] = fill_value
             else:
-                codes[periods:] = -1
+                codes[periods:] = fill_value
 
         return self.from_codes(codes, categories=self.categories,
                                ordered=self.ordered)
diff --git a/pandas/core/arrays/period.py b/pandas/core/arrays/period.py
index d9dde1c699761..5a819d11080db 100644
--- a/pandas/core/arrays/period.py
+++ b/pandas/core/arrays/period.py
@@ -457,7 +457,7 @@ def value_counts(self, dropna=False):
 
     # --------------------------------------------------------------------
 
-    def shift(self, periods=1):
+    def shift(self, periods=1, fill_value=None):
         """
         Shift values by desired number.
 
@@ -471,6 +471,8 @@ def shift(self, periods=1):
         periods : int, default 1
             The number of periods to shift. Negative values are allowed
             for shifting backwards.
+        fill_value : None or value, default None (NaT)
+            .. versionadded:: 0.24.0
 
         Returns
         -------
@@ -479,7 +481,7 @@ def shift(self, periods=1):
         # TODO(DatetimeArray): remove
         # The semantics for Index.shift differ from EA.shift
         # then just call super.
-        return ExtensionArray.shift(self, periods)
+        return ExtensionArray.shift(self, periods, fill_value=fill_value)
 
     def _time_shift(self, n, freq=None):
         """
diff --git a/pandas/core/arrays/sparse.py b/pandas/core/arrays/sparse.py
index 0bd899149d940..8352fee3e32aa 100644
--- a/pandas/core/arrays/sparse.py
+++ b/pandas/core/arrays/sparse.py
@@ -887,7 +887,7 @@ def fillna(self, value=None, method=None, limit=None):
 
         return self._simple_new(new_values, self._sparse_index, new_dtype)
 
-    def shift(self, periods=1):
+    def shift(self, periods=1, fill_value=None):
 
         if not len(self) or periods == 0:
             return self.copy()
@@ -900,8 +900,11 @@ def shift(self, periods=1):
         else:
             arr = self
 
+        if fill_value is None:
+            fill_value = self.dtype.na_value
+
         empty = self._from_sequence(
-            [self.dtype.na_value] * min(abs(periods), len(self)),
+            [fill_value] * min(abs(periods), len(self)),
             dtype=arr.dtype
         )
 
diff --git a/pandas/core/frame.py b/pandas/core/frame.py
index f7f1855a4fabc..aee98999405c7 100644
--- a/pandas/core/frame.py
+++ b/pandas/core/frame.py
@@ -3867,9 +3867,9 @@ def replace(self, to_replace=None, value=None, inplace=False, limit=None,
                                               method=method)
 
     @Appender(_shared_docs['shift'] % _shared_doc_kwargs)
-    def shift(self, periods=1, freq=None, axis=0):
+    def shift(self, periods=1, freq=None, axis=0, fill_value=None):
         return super(DataFrame, self).shift(periods=periods, freq=freq,
-                                            axis=axis)
+                                            axis=axis, fill_value=fill_value)
 
     def set_index(self, keys, drop=True, append=False, inplace=False,
                   verify_integrity=False):
diff --git a/pandas/core/generic.py b/pandas/core/generic.py
index 1e26c3f45f660..13e71c566511b 100644
--- a/pandas/core/generic.py
+++ b/pandas/core/generic.py
@@ -8839,6 +8839,12 @@ def mask(self, cond, other=np.nan, inplace=False, axis=None, level=None,
             extend the index when shifting and preserve the original data.
         axis : {0 or 'index', 1 or 'columns', None}, default None
             Shift direction.
+        fill_value : object, optional
+            the scalar value to use for newly introduced missing values.
+            the default depends on the dtype of `self`. for numeric data,
+            ``np.nan`` is used. for datelike, ``pandas.nat`` is used.
+
+            .. versionchanged:: 0.24.0
 
         Returns
         -------
@@ -8874,16 +8880,25 @@ def mask(self, cond, other=np.nan, inplace=False, axis=None, level=None,
         2   NaN  15.0  18.0
         3   NaN  30.0  33.0
         4   NaN  45.0  48.0
+
+        >>> df.shift(periods=3, fill_value=0.0)
+           Col1  Col2  Col3
+        0   0.0   0.0   0.0
+        1   0.0   0.0   0.0
+        2   0.0   0.0   0.0
+        3  10.0  13.0  17.0
+        4  20.0  23.0  27.0
     """)
 
     @Appender(_shared_docs['shift'] % _shared_doc_kwargs)
-    def shift(self, periods=1, freq=None, axis=0):
+    def shift(self, periods=1, freq=None, axis=0, fill_value=None):
         if periods == 0:
             return self.copy()
 
         block_axis = self._get_block_manager_axis(axis)
         if freq is None:
-            new_data = self._data.shift(periods=periods, axis=block_axis)
+            new_data = self._data.shift(periods=periods, axis=block_axis,
+                                        fill_value=fill_value)
         else:
             return self.tshift(periods, freq)
 
diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py
index 45eaa3efa948a..e6d7dc29b2d30 100644
--- a/pandas/core/groupby/groupby.py
+++ b/pandas/core/groupby/groupby.py
@@ -1992,7 +1992,7 @@ def _get_cythonized_result(self, how, grouper, aggregate=False,
 
     @Substitution(name='groupby')
     @Appender(_doc_template)
-    def shift(self, periods=1, freq=None, axis=0):
+    def shift(self, periods=1, freq=None, axis=0, fill_value=None):
         """
         Shift each group by periods observations.
 
@@ -2002,10 +2002,12 @@ def shift(self, periods=1, freq=None, axis=0):
             number of periods to shift
         freq : frequency string
         axis : axis to shift, default 0
+        fill_value : None or value, default None (NaN)
         """
 
         if freq is not None or axis != 0:
-            return self.apply(lambda x: x.shift(periods, freq, axis))
+            return self.apply(lambda x: x.shift(periods, freq,
+                                                axis, fill_value))
 
         return self._get_cythonized_result('group_shift_indexer',
                                            self.grouper, cython_dtype=np.int64,
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index d37da14ab5d2c..eb9177e455ecb 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -1260,12 +1260,12 @@ def diff(self, n, axis=1):
         new_values = algos.diff(self.values, n, axis=axis)
         return [self.make_block(values=new_values)]
 
-    def shift(self, periods, axis=0):
+    def shift(self, periods, axis=0, fill_value=None):
         """ shift the block by periods, possibly upcast """
 
         # convert integer to float if necessary. need to do a lot more than
         # that, handle boolean etc also
-        new_values, fill_value = maybe_upcast(self.values)
+        new_values, fill_value = maybe_upcast(self.values, fill_value)
 
         # make sure array sent to np.roll is c_contiguous
         f_ordered = new_values.flags.f_contiguous
@@ -1955,7 +1955,7 @@ def interpolate(self, method='pad', axis=0, inplace=False, limit=None,
                                  limit=limit),
             placement=self.mgr_locs)
 
-    def shift(self, periods, axis=0):
+    def shift(self, periods, axis=0, fill_value=None):
         """
         Shift the block by `periods`.
 
@@ -1963,9 +1963,11 @@ def shift(self, periods, axis=0):
         ExtensionBlock.
         """
         # type: (int, Optional[BlockPlacement]) -> List[ExtensionBlock]
-        return [self.make_block_same_class(self.values.shift(periods=periods),
-                                           placement=self.mgr_locs,
-                                           ndim=self.ndim)]
+        return [
+            self.make_block_same_class(
+                self.values.shift(periods=periods, fill_value=fill_value),
+                placement=self.mgr_locs, ndim=self.ndim)
+        ]
 
     @property
     def _ftype(self):
@@ -2945,7 +2947,7 @@ def _try_coerce_result(self, result):
     def _box_func(self):
         return lambda x: tslibs.Timestamp(x, tz=self.dtype.tz)
 
-    def shift(self, periods, axis=0):
+    def shift(self, periods, axis=0, fill_value=None):
         """ shift the block by periods """
 
         # think about moving this to the DatetimeIndex. This is a non-freq
@@ -2960,10 +2962,12 @@ def shift(self, periods, axis=0):
 
         new_values = self.values.asi8.take(indexer)
 
+        if fill_value is None:
+            fill_value = tslibs.iNaT
         if periods > 0:
-            new_values[:periods] = tslibs.iNaT
+            new_values[:periods] = fill_value
         else:
-            new_values[periods:] = tslibs.iNaT
+            new_values[periods:] = fill_value
 
         new_values = self.values._shallow_copy(new_values)
         return [self.make_block_same_class(new_values,
diff --git a/pandas/core/series.py b/pandas/core/series.py
index 4f9465354a47b..6ce5a70b72f95 100644
--- a/pandas/core/series.py
+++ b/pandas/core/series.py
@@ -3714,8 +3714,9 @@ def replace(self, to_replace=None, value=None, inplace=False, limit=None,
                                            regex=regex, method=method)
 
     @Appender(generic._shared_docs['shift'] % _shared_doc_kwargs)
-    def shift(self, periods=1, freq=None, axis=0):
-        return super(Series, self).shift(periods=periods, freq=freq, axis=axis)
+    def shift(self, periods=1, freq=None, axis=0, fill_value=None):
+        return super(Series, self).shift(periods=periods, freq=freq, axis=axis,
+                                         fill_value=fill_value)
 
     def reindex_axis(self, labels, axis=0, **kwargs):
         """
diff --git a/pandas/tests/frame/test_timeseries.py b/pandas/tests/frame/test_timeseries.py
index 52f0b30bf0f0c..02b83aaf5c131 100644
--- a/pandas/tests/frame/test_timeseries.py
+++ b/pandas/tests/frame/test_timeseries.py
@@ -320,6 +320,20 @@ def test_shift_categorical(self):
         xp = DataFrame({'one': s1.shift(1), 'two': s2.shift(1)})
         assert_frame_equal(rs, xp)
 
+    def test_shift_fill_value(self):
+        # GH #24128
+        df = DataFrame([1, 2, 3, 4, 5],
+                       index=date_range('1/1/2000', periods=5, freq='H'))
+        exp = DataFrame([0, 1, 2, 3, 4],
+                        index=date_range('1/1/2000', periods=5, freq='H'))
+        result = df.shift(1, fill_value=0)
+        assert_frame_equal(result, exp)
+
+        exp = DataFrame([0, 0, 1, 2, 3],
+                        index=date_range('1/1/2000', periods=5, freq='H'))
+        result = df.shift(2, fill_value=0)
+        assert_frame_equal(result, exp)
+
     def test_shift_empty(self):
         # Regression test for #8019
         df = DataFrame({'foo': []})
diff --git a/pandas/tests/series/test_timeseries.py b/pandas/tests/series/test_timeseries.py
index b9cf845ea47d7..aab378d9399d6 100644
--- a/pandas/tests/series/test_timeseries.py
+++ b/pandas/tests/series/test_timeseries.py
@@ -129,6 +129,28 @@ def test_shift2(self):
         idx = DatetimeIndex(['2000-01-01', '2000-01-02', '2000-01-04'])
         pytest.raises(NullFrequencyError, idx.shift, 1)
 
+    def test_shift_fill_value(self):
+        # GH #24128
+        ts = Series([1.0, 2.0, 3.0, 4.0, 5.0],
+                    index=date_range('1/1/2000', periods=5, freq='H'))
+
+        # fill_value should have no effect on shift with freq
+        result = ts.shift(1, freq='5T', fill_value=0)
+        exp_index = ts.index.shift(1, freq='5T')
+        tm.assert_index_equal(result.index, exp_index)
+        tm.assert_numpy_array_equal(result.values, ts.values)
+
+        exp = Series([0.0, 1.0, 2.0, 3.0, 4.0],
+                     index=date_range('1/1/2000', periods=5, freq='H'))
+        # check that fill value works
+        result = ts.shift(1, fill_value=0.0)
+        tm.assert_series_equal(result, exp)
+
+        exp = Series([0.0, 0.0, 1.0, 2.0, 3.0],
+                     index=date_range('1/1/2000', periods=5, freq='H'))
+        result = ts.shift(2, fill_value=0.0)
+        tm.assert_series_equal(result, exp)
+
     def test_shift_dst(self):
         # GH 13926
         dates = date_range('2016-11-06', freq='H', periods=10, tz='US/Eastern')


Code review comments:
diff_hunk: @@ -900,8 +900,11 @@ def shift(self, periods=1):
         else:
             arr = self
 
+        if fill_value is None:
path: pandas/core/arrays/sparse.py
text: @user1:
is this not the same as the arrays/base.py impl? do we need this override at all?

@author:
there is a small diff in those lines

        subtype = np.result_type(np.nan, self.dtype.subtype)

        if subtype != self.dtype.subtype:
            # just coerce up front
            arr = self.astype(SparseDtype(subtype, self.fill_value))
        else:
            arr = self
if I see correctly

@user1:
so you can do this in the Sparse version, then call the super method no (and pass in the dtype)

@author:
yes, but that would require a new argument for the method in the base class, and therefore changing the signature of the method and all the versions of it in subclasses. If it is not a problem I would like to avoid adding more changes to PR if they are not required.
I can make those change as an independent PR if that works for you

@author:
@user1 please let me know what you think

diff_hunk: @@ -471,6 +471,8 @@ def shift(self, periods=1):
         periods : int, default 1
             The number of periods to shift. Negative values are allowed
             for shifting backwards.
+        fill_value : None or value, default None (NaT)
path: pandas/core/arrays/period.py
text: @user1:
optional, default NaT

@author:
changed