From 049a6fac22505be91a9a90ff9d3ea106f9580149 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler <61934744+phofl@users.noreply.github.com> Date: Mon, 28 Aug 2023 19:33:42 +0200 Subject: [PATCH 1/4] DEP: Deprecate passing fill_value and freq to shift --- doc/source/whatsnew/v2.1.0.rst | 2 +- pandas/core/frame.py | 9 ++++++--- pandas/core/generic.py | 9 ++++++--- pandas/tests/frame/methods/test_shift.py | 12 ++++++++---- pandas/tests/groupby/test_groupby_shift_diff.py | 8 +++++--- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/doc/source/whatsnew/v2.1.0.rst b/doc/source/whatsnew/v2.1.0.rst index 8f2667d69a322..fbf5a6c8d2da8 100644 --- a/doc/source/whatsnew/v2.1.0.rst +++ b/doc/source/whatsnew/v2.1.0.rst @@ -587,6 +587,7 @@ Other Deprecations - Deprecated values ``"pad"``, ``"ffill"``, ``"bfill"``, ``"backfill"`` for :meth:`Series.interpolate` and :meth:`DataFrame.interpolate`, use ``obj.ffill()`` or ``obj.bfill()`` instead (:issue:`53581`) - Deprecated the behavior of :meth:`Index.argmax`, :meth:`Index.argmin`, :meth:`Series.argmax`, :meth:`Series.argmin` with either all-NAs and ``skipna=True`` or any-NAs and ``skipna=False`` returning -1; in a future version this will raise ``ValueError`` (:issue:`33941`, :issue:`33942`) - Deprecated allowing non-keyword arguments in :meth:`DataFrame.to_sql` except ``name`` and ``con`` (:issue:`54229`) +- Deprecated silently ignoring ``fill_value`` when passing both ``freq`` and ``fill_value`` to :meth:`DataFrame.shift` and :meth:`Series.shift` and :meth:`.DataFrameGroupBy.shift`; in a future version this will raise ``ValueError`` (:issue:`53832`) .. --------------------------------------------------------------------------- .. _whatsnew_210.performance: @@ -871,7 +872,6 @@ Other - Bug in :meth:`.DataFrameGroupBy.first`, :meth:`.DataFrameGroupBy.last`, :meth:`.SeriesGroupBy.first`, and :meth:`.SeriesGroupBy.last` where an empty group would return ``np.nan`` instead of the corresponding :class:`.ExtensionArray` NA value (:issue:`39098`) - Bug in :meth:`DataFrame.pivot_table` with casting the mean of ints back to an int (:issue:`16676`) - Bug in :meth:`DataFrame.reindex` with a ``fill_value`` that should be inferred with a :class:`ExtensionDtype` incorrectly inferring ``object`` dtype (:issue:`52586`) -- Bug in :meth:`DataFrame.shift` and :meth:`Series.shift` and :meth:`.DataFrameGroupBy.shift` when passing both ``freq`` and ``fill_value`` silently ignoring ``fill_value`` instead of raising ``ValueError`` (:issue:`53832`) - Bug in :meth:`DataFrame.shift` with ``axis=1`` on a :class:`DataFrame` with a single :class:`ExtensionDtype` column giving incorrect results (:issue:`53832`) - Bug in :meth:`Index.sort_values` when a ``key`` is passed (:issue:`52764`) - Bug in :meth:`Series.align`, :meth:`DataFrame.align`, :meth:`Series.reindex`, :meth:`DataFrame.reindex`, :meth:`Series.interpolate`, :meth:`DataFrame.interpolate`, incorrectly failing to raise with method="asfreq" (:issue:`53620`) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 018059ce2784f..72a43c3d98418 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -5636,10 +5636,13 @@ def shift( ) -> DataFrame: if freq is not None and fill_value is not lib.no_default: # GH#53832 - raise ValueError( - "Cannot pass both 'freq' and 'fill_value' to " - f"{type(self).__name__}.shift" + warnings.warn( + "Passing a 'freq' together with a 'fill_value' silently ignores " + "the fill_value. This will raise in a future version.", + FutureWarning, + stacklevel=find_stack_level(), ) + fill_value = lib.no_default axis = self._get_axis_number(axis) diff --git a/pandas/core/generic.py b/pandas/core/generic.py index b87772594cfbf..23173150a8ed8 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -10822,10 +10822,13 @@ def shift( if freq is not None and fill_value is not lib.no_default: # GH#53832 - raise ValueError( - "Cannot pass both 'freq' and 'fill_value' to " - f"{type(self).__name__}.shift" + warnings.warn( + "Passing a 'freq' together with a 'fill_value' silently ignores " + "the fill_value. This will raise in a future version.", + FutureWarning, + stacklevel=find_stack_level(), ) + fill_value = lib.no_default if periods == 0: return self.copy(deep=None) diff --git a/pandas/tests/frame/methods/test_shift.py b/pandas/tests/frame/methods/test_shift.py index 808f0cff2485c..54249f3db509c 100644 --- a/pandas/tests/frame/methods/test_shift.py +++ b/pandas/tests/frame/methods/test_shift.py @@ -32,19 +32,23 @@ def test_shift_axis1_with_valid_fill_value_one_array(self): expected2 = DataFrame([12345] * 5, dtype="Float64") tm.assert_frame_equal(res2, expected2) - def test_shift_disallow_freq_and_fill_value(self, frame_or_series): + def test_shift_deprecate_freq_and_fill_value(self, frame_or_series): # Can't pass both! obj = frame_or_series( np.random.default_rng(2).standard_normal(5), index=date_range("1/1/2000", periods=5, freq="H"), ) - msg = "Cannot pass both 'freq' and 'fill_value' to (Series|DataFrame).shift" - with pytest.raises(ValueError, match=msg): + msg = ( + "Passing a 'freq' together with a 'fill_value' silently ignores the " + "fill_value" + ) + with tm.assert_produces_warning(FutureWarning, match=msg): obj.shift(1, fill_value=1, freq="H") if frame_or_series is DataFrame: - with pytest.raises(ValueError, match=msg): + frame_or_series.columns = date_range("1/1/2000", periods=1, freq="H") + with tm.assert_produces_warning(FutureWarning, match=msg): obj.shift(1, axis=1, fill_value=1, freq="H") @pytest.mark.parametrize( diff --git a/pandas/tests/groupby/test_groupby_shift_diff.py b/pandas/tests/groupby/test_groupby_shift_diff.py index 495f3fcd359c7..21a030ca276d4 100644 --- a/pandas/tests/groupby/test_groupby_shift_diff.py +++ b/pandas/tests/groupby/test_groupby_shift_diff.py @@ -166,12 +166,14 @@ def test_shift_periods_freq(): tm.assert_frame_equal(result, expected) -def test_shift_disallow_freq_and_fill_value(): +def test_shift_deprecate_freq_and_fill_value(): # GH 53832 data = {"a": [1, 2, 3, 4, 5, 6], "b": [0, 0, 0, 1, 1, 1]} df = DataFrame(data, index=date_range(start="20100101", periods=6)) - msg = "Cannot pass both 'freq' and 'fill_value' to (Series|DataFrame).shift" - with pytest.raises(ValueError, match=msg): + msg = ( + "Passing a 'freq' together with a 'fill_value' silently ignores the fill_value" + ) + with tm.assert_produces_warning(FutureWarning, match=msg): df.groupby(df.index).shift(periods=-2, freq="D", fill_value="1") From 10cac2425d3237996d2b68582af9c59a6d52de2a Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Mon, 28 Aug 2023 23:06:59 +0200 Subject: [PATCH 2/4] Update --- doc/source/whatsnew/v2.1.0.rst | 2 +- pandas/core/frame.py | 3 ++- pandas/core/generic.py | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v2.1.0.rst b/doc/source/whatsnew/v2.1.0.rst index fbf5a6c8d2da8..e7fc83c35f56d 100644 --- a/doc/source/whatsnew/v2.1.0.rst +++ b/doc/source/whatsnew/v2.1.0.rst @@ -587,7 +587,7 @@ Other Deprecations - Deprecated values ``"pad"``, ``"ffill"``, ``"bfill"``, ``"backfill"`` for :meth:`Series.interpolate` and :meth:`DataFrame.interpolate`, use ``obj.ffill()`` or ``obj.bfill()`` instead (:issue:`53581`) - Deprecated the behavior of :meth:`Index.argmax`, :meth:`Index.argmin`, :meth:`Series.argmax`, :meth:`Series.argmin` with either all-NAs and ``skipna=True`` or any-NAs and ``skipna=False`` returning -1; in a future version this will raise ``ValueError`` (:issue:`33941`, :issue:`33942`) - Deprecated allowing non-keyword arguments in :meth:`DataFrame.to_sql` except ``name`` and ``con`` (:issue:`54229`) -- Deprecated silently ignoring ``fill_value`` when passing both ``freq`` and ``fill_value`` to :meth:`DataFrame.shift` and :meth:`Series.shift` and :meth:`.DataFrameGroupBy.shift`; in a future version this will raise ``ValueError`` (:issue:`53832`) +- Deprecated silently ignoring ``fill_value`` when passing both ``freq`` and ``fill_value`` to :meth:`DataFrame.shift`, :meth:`Series.shift` and :meth:`.DataFrameGroupBy.shift`; in a future version this will raise ``ValueError`` (:issue:`53832`) .. --------------------------------------------------------------------------- .. _whatsnew_210.performance: diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 72a43c3d98418..997e13e3f0503 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -5638,7 +5638,8 @@ def shift( # GH#53832 warnings.warn( "Passing a 'freq' together with a 'fill_value' silently ignores " - "the fill_value. This will raise in a future version.", + "the fill_value and is deprecated. This will raise in a future " + "version.", FutureWarning, stacklevel=find_stack_level(), ) diff --git a/pandas/core/generic.py b/pandas/core/generic.py index 23173150a8ed8..d6ee8ad2cd347 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -10824,7 +10824,8 @@ def shift( # GH#53832 warnings.warn( "Passing a 'freq' together with a 'fill_value' silently ignores " - "the fill_value. This will raise in a future version.", + "the fill_value and is deprecated. This will raise in a future " + "version.", FutureWarning, stacklevel=find_stack_level(), ) From 6c81afc48a05dcefe1eaa214b7dc5048d932ba26 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Tue, 29 Aug 2023 00:36:58 +0200 Subject: [PATCH 3/4] Fix --- pandas/tests/frame/methods/test_shift.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pandas/tests/frame/methods/test_shift.py b/pandas/tests/frame/methods/test_shift.py index 54249f3db509c..1e88152137b1f 100644 --- a/pandas/tests/frame/methods/test_shift.py +++ b/pandas/tests/frame/methods/test_shift.py @@ -47,7 +47,7 @@ def test_shift_deprecate_freq_and_fill_value(self, frame_or_series): obj.shift(1, fill_value=1, freq="H") if frame_or_series is DataFrame: - frame_or_series.columns = date_range("1/1/2000", periods=1, freq="H") + obj.columns = date_range("1/1/2000", periods=1, freq="H") with tm.assert_produces_warning(FutureWarning, match=msg): obj.shift(1, axis=1, fill_value=1, freq="H") @@ -720,8 +720,11 @@ def test_shift_with_iterable_freq_and_fill_value(self): df.shift(1, freq="H"), ) - msg = r"Cannot pass both 'freq' and 'fill_value' to.*" - with pytest.raises(ValueError, match=msg): + msg = ( + "Passing a 'freq' together with a 'fill_value' silently ignores the " + "fill_value" + ) + with tm.assert_produces_warning(FutureWarning, match=msg): df.shift([1, 2], fill_value=1, freq="H") def test_shift_with_iterable_check_other_arguments(self): From 56e0aa663936dc07d857db9b7a94dbf98e9924ac Mon Sep 17 00:00:00 2001 From: Patrick Hoefler <61934744+phofl@users.noreply.github.com> Date: Tue, 29 Aug 2023 14:46:58 +0200 Subject: [PATCH 4/4] Fix test --- pandas/tests/groupby/test_groupby_shift_diff.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pandas/tests/groupby/test_groupby_shift_diff.py b/pandas/tests/groupby/test_groupby_shift_diff.py index 21a030ca276d4..bb4b9aa866ac9 100644 --- a/pandas/tests/groupby/test_groupby_shift_diff.py +++ b/pandas/tests/groupby/test_groupby_shift_diff.py @@ -240,12 +240,15 @@ def test_group_shift_with_multiple_periods_and_fill_value(): tm.assert_frame_equal(shifted_df, expected_df) -def test_group_shift_with_multiple_periods_and_both_fill_and_freq_fails(): +def test_group_shift_with_multiple_periods_and_both_fill_and_freq_deprecated(): # GH#44424 df = DataFrame( {"a": [1, 2, 3, 4, 5], "b": [True, True, False, False, True]}, index=date_range("1/1/2000", periods=5, freq="H"), ) - msg = r"Cannot pass both 'freq' and 'fill_value' to.*" - with pytest.raises(ValueError, match=msg): + msg = ( + "Passing a 'freq' together with a 'fill_value' silently ignores the " + "fill_value" + ) + with tm.assert_produces_warning(FutureWarning, match=msg): df.groupby("b")[["a"]].shift([1, 2], fill_value=1, freq="H")