Skip to content

Commit

Permalink
Backport PR #54818 on branch 2.1.x (DEP: Deprecate passing fill_value…
Browse files Browse the repository at this point in the history
… and freq to shift) (#54849)

Backport PR #54818: DEP: Deprecate passing fill_value and freq to shift

Co-authored-by: Patrick Hoefler <61934744+phofl@users.noreply.github.com>
  • Loading branch information
meeseeksmachine and phofl committed Aug 29, 2023
1 parent 3b7f411 commit 772beaa
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 19 deletions.
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v2.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,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`, :meth:`Series.shift` and :meth:`.DataFrameGroupBy.shift`; in a future version this will raise ``ValueError`` (:issue:`53832`)

.. ---------------------------------------------------------------------------
.. _whatsnew_210.performance:
Expand Down Expand Up @@ -875,7 +876,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`)
Expand Down
10 changes: 7 additions & 3 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -5617,10 +5617,14 @@ 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 and is deprecated. This will raise in a future "
"version.",
FutureWarning,
stacklevel=find_stack_level(),
)
fill_value = lib.no_default

axis = self._get_axis_number(axis)

Expand Down
10 changes: 7 additions & 3 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -10802,10 +10802,14 @@ 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 and is deprecated. 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)
Expand Down
19 changes: 13 additions & 6 deletions pandas/tests/frame/methods/test_shift.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
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")

@pytest.mark.parametrize(
Expand Down Expand Up @@ -716,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):
Expand Down
17 changes: 11 additions & 6 deletions pandas/tests/groupby/test_groupby_shift_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")


Expand Down Expand Up @@ -238,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")

0 comments on commit 772beaa

Please sign in to comment.