Skip to content

CLN: enforce deprecation of NDFrame.interpolate with ffill/bfill/pad/backfill methods #57869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 22, 2024
Merged

CLN: enforce deprecation of NDFrame.interpolate with ffill/bfill/pad/backfill methods #57869

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b815305
enforce deprecation of interpolate with ffill, bfill-pad, backfill me…
natmokval Mar 16, 2024
5e767cb
remove redundant if branch
natmokval Mar 18, 2024
e9502e2
remove unuseful cheek from interpolate
natmokval Mar 18, 2024
97d951c
move checking for a fillna_method from NDFrame.interpolate to Block.i…
natmokval Mar 19, 2024
389ffb9
remove the check from Block.interpolate
natmokval Mar 21, 2024
4582e0c
Merge branch 'main' into cln-enforce-deprecation-interpolate-with-met…
natmokval Mar 21, 2024
9a46c68
add a note to v3.0.0
natmokval Mar 21, 2024
9ddb084
correct def _interpolate_scipy_wrapper: use alt_methods instead of valid
natmokval Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 3 additions & 23 deletions pandas/core/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7630,7 +7630,6 @@ def interpolate(
* 'time': Works on daily and higher resolution data to interpolate
given length of interval.
* 'index', 'values': use the actual numerical values of the index.
* 'pad': Fill in NaNs using existing values.
* 'nearest', 'zero', 'slinear', 'quadratic', 'cubic',
'barycentric', 'polynomial': Passed to
`scipy.interpolate.interp1d`, whereas 'spline' is passed to
Expand All @@ -7654,23 +7653,9 @@ def interpolate(
0.
inplace : bool, default False
Update the data in place if possible.
limit_direction : {{'forward', 'backward', 'both'}}, Optional
limit_direction : {{'forward', 'backward', 'both'}}, optional, default 'forward'
Consecutive NaNs will be filled in this direction.

If limit is specified:
* If 'method' is 'pad' or 'ffill', 'limit_direction' must be 'forward'.
* If 'method' is 'backfill' or 'bfill', 'limit_direction' must be
'backwards'.

If 'limit' is not specified:
* If 'method' is 'backfill' or 'bfill', the default is 'backward'
* else the default is 'forward'

raises ValueError if `limit_direction` is 'forward' or 'both' and
method is 'backfill' or 'bfill'.
raises ValueError if `limit_direction` is 'backward' or 'both' and
method is 'pad' or 'ffill'.

limit_area : {{`None`, 'inside', 'outside'}}, default None
If limit is specified, consecutive NaNs will be filled with this
restriction.
Expand Down Expand Up @@ -7806,14 +7791,9 @@ def interpolate(
fillna_methods = ["ffill", "bfill", "pad", "backfill"]
if method.lower() in fillna_methods:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just remove this entire if branch? A bad method should raise later on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing this PR. I removed this if branch and changed the error message in the if branch below. Just one question: we didn't have a test for the previous ValueError message:

"'fill_value' is not a valid keyword for "
f"{type(self).__name__}.interpolate with method from "
f"{fillna_methods}"

Should I add the test?

# GH#53581
warnings.warn(
f"{type(self).__name__}.interpolate with method={method} is "
"deprecated and will raise in a future version. "
"Use obj.ffill() or obj.bfill() instead.",
FutureWarning,
stacklevel=find_stack_level(),
raise ValueError(
f"{type(self).__name__} cannot interpolate with method={method}."
)
obj, should_transpose = self, False
else:
obj, should_transpose = (self.T, True) if axis == 1 else (self, False)
# GH#53631
Expand Down
34 changes: 17 additions & 17 deletions pandas/tests/copy_view/test_interp_fillna.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,18 @@ def test_interpolate_no_op(method):
df = DataFrame({"a": [1, 2]})
df_orig = df.copy()

warn = None
if method == "pad":
warn = FutureWarning
msg = "DataFrame.interpolate with method=pad is deprecated"
with tm.assert_produces_warning(warn, match=msg):
msg = f"DataFrame cannot interpolate with method={method}"
with pytest.raises(ValueError, match=msg):
df.interpolate(method=method)
else:
result = df.interpolate(method=method)
assert np.shares_memory(get_array(result, "a"), get_array(df, "a"))

assert np.shares_memory(get_array(result, "a"), get_array(df, "a"))
result.iloc[0, 0] = 100

result.iloc[0, 0] = 100

assert not np.shares_memory(get_array(result, "a"), get_array(df, "a"))
tm.assert_frame_equal(df, df_orig)
assert not np.shares_memory(get_array(result, "a"), get_array(df, "a"))
tm.assert_frame_equal(df, df_orig)


@pytest.mark.parametrize("func", ["ffill", "bfill"])
Expand Down Expand Up @@ -122,8 +121,8 @@ def test_interpolate_cannot_with_object_dtype():
def test_interpolate_object_convert_no_op():
df = DataFrame({"a": ["a", "b", "c"], "b": 1})
arr_a = get_array(df, "a")
msg = "DataFrame.interpolate with method=pad is deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):
msg = "DataFrame cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
df.interpolate(method="pad", inplace=True)

# Now CoW makes a copy, it should not!
Expand All @@ -134,8 +133,8 @@ def test_interpolate_object_convert_no_op():
def test_interpolate_object_convert_copies():
df = DataFrame({"a": [1, np.nan, 2.5], "b": 1})
arr_a = get_array(df, "a")
msg = "DataFrame.interpolate with method=pad is deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):
msg = "DataFrame cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
df.interpolate(method="pad", inplace=True, downcast="infer")

assert df._mgr._has_no_reference(0)
Expand All @@ -147,12 +146,13 @@ def test_interpolate_downcast_reference_triggers_copy():
df_orig = df.copy()
arr_a = get_array(df, "a")
view = df[:]
msg = "DataFrame.interpolate with method=pad is deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):

msg = "DataFrame cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
df.interpolate(method="pad", inplace=True, downcast="infer")
assert df._mgr._has_no_reference(0)
assert not np.shares_memory(arr_a, get_array(df, "a"))

assert df._mgr._has_no_reference(0)
assert not np.shares_memory(arr_a, get_array(df, "a"))
tm.assert_frame_equal(df_orig, view)


Expand Down
9 changes: 3 additions & 6 deletions pandas/tests/frame/methods/test_interpolate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,9 @@ def test_interp_fillna_methods(self, axis, multiblock, method):
df["D"] = np.nan
df["E"] = 1.0

method2 = method if method != "pad" else "ffill"
expected = getattr(df, method2)(axis=axis)
msg = f"DataFrame.interpolate with method={method} is deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):
result = df.interpolate(method=method, axis=axis)
tm.assert_frame_equal(result, expected)
msg = f"DataFrame cannot interpolate with method={method}"
with pytest.raises(ValueError, match=msg):
df.interpolate(method=method, axis=axis)

def test_interpolate_empty_df(self):
# GH#53199
Expand Down
104 changes: 28 additions & 76 deletions pandas/tests/series/methods/test_interpolate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,9 @@ def test_interp_invalid_method_and_value(self):
# GH#36624
ser = Series([1, 3, np.nan, 12, np.nan, 25])

msg = "'fill_value' is not a valid keyword for Series.interpolate"
msg2 = "Series.interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
with tm.assert_produces_warning(FutureWarning, match=msg2):
ser.interpolate(fill_value=3, method="pad")
msg2 = "Series cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg2):
ser.interpolate(fill_value=3, method="pad")

def test_interp_limit_forward(self):
s = Series([1, 3, np.nan, np.nan, np.nan, 11])
Expand Down Expand Up @@ -455,107 +453,70 @@ def test_interp_limit_area(self):
s.interpolate(method="linear", limit_area="abc")

@pytest.mark.parametrize(
"method, limit_direction, expected",
[
("pad", "backward", "forward"),
("ffill", "backward", "forward"),
("backfill", "forward", "backward"),
("bfill", "forward", "backward"),
("pad", "both", "forward"),
("ffill", "both", "forward"),
("backfill", "both", "backward"),
("bfill", "both", "backward"),
],
)
def test_interp_limit_direction_raises(self, method, limit_direction, expected):
# https://github.com/pandas-dev/pandas/pull/34746
s = Series([1, 2, 3])

msg = f"`limit_direction` must be '{expected}' for method `{method}`"
msg2 = "Series.interpolate with method="
with pytest.raises(ValueError, match=msg):
with tm.assert_produces_warning(FutureWarning, match=msg2):
s.interpolate(method=method, limit_direction=limit_direction)

@pytest.mark.parametrize(
"data, expected_data, kwargs",
"data, kwargs",
(
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, np.nan, 3.0, 3.0, 3.0, 3.0, 7.0, np.nan, np.nan],
{"method": "pad", "limit_area": "inside"},
),
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, np.nan, 3.0, 3.0, np.nan, np.nan, 7.0, np.nan, np.nan],
{"method": "pad", "limit_area": "inside", "limit": 1},
),
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, np.nan, 3.0, np.nan, np.nan, np.nan, 7.0, 7.0, 7.0],
{"method": "pad", "limit_area": "outside"},
),
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, np.nan, 3.0, np.nan, np.nan, np.nan, 7.0, 7.0, np.nan],
{"method": "pad", "limit_area": "outside", "limit": 1},
),
(
[np.nan, np.nan, np.nan, np.nan, np.nan, np.nan, np.nan],
[np.nan, np.nan, np.nan, np.nan, np.nan, np.nan, np.nan],
{"method": "pad", "limit_area": "outside", "limit": 1},
),
(
range(5),
range(5),
{"method": "pad", "limit_area": "outside", "limit": 1},
),
),
)
def test_interp_limit_area_with_pad(self, data, expected_data, kwargs):
def test_interp_limit_area_with_pad(self, data, kwargs):
# GH26796

s = Series(data)
expected = Series(expected_data)
msg = "Series.interpolate with method=pad"
with tm.assert_produces_warning(FutureWarning, match=msg):
result = s.interpolate(**kwargs)
tm.assert_series_equal(result, expected)
msg = "Series cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
s.interpolate(**kwargs)

@pytest.mark.parametrize(
"data, expected_data, kwargs",
"data, kwargs",
(
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, np.nan, 3.0, 7.0, 7.0, 7.0, 7.0, np.nan, np.nan],
{"method": "bfill", "limit_area": "inside"},
),
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, np.nan, 3.0, np.nan, np.nan, 7.0, 7.0, np.nan, np.nan],
{"method": "bfill", "limit_area": "inside", "limit": 1},
),
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[3.0, 3.0, 3.0, np.nan, np.nan, np.nan, 7.0, np.nan, np.nan],
{"method": "bfill", "limit_area": "outside"},
),
(
[np.nan, np.nan, 3, np.nan, np.nan, np.nan, 7, np.nan, np.nan],
[np.nan, 3.0, 3.0, np.nan, np.nan, np.nan, 7.0, np.nan, np.nan],
{"method": "bfill", "limit_area": "outside", "limit": 1},
),
),
)
def test_interp_limit_area_with_backfill(self, data, expected_data, kwargs):
def test_interp_limit_area_with_backfill(self, data, kwargs):
# GH26796

s = Series(data)
expected = Series(expected_data)
msg = "Series.interpolate with method=bfill"
with tm.assert_produces_warning(FutureWarning, match=msg):
result = s.interpolate(**kwargs)
tm.assert_series_equal(result, expected)

msg = "Series cannot interpolate with method=bfill"
with pytest.raises(ValueError, match=msg):
s.interpolate(**kwargs)

def test_interp_limit_direction(self):
# These tests are for issue #9218 -- fill NaNs in both directions.
Expand Down Expand Up @@ -650,37 +611,28 @@ def test_interp_datetime64(self, method, tz_naive_fixture):
df = Series(
[1, np.nan, 3], index=date_range("1/1/2000", periods=3, tz=tz_naive_fixture)
)
warn = None if method == "nearest" else FutureWarning
msg = "Series.interpolate with method=pad is deprecated"
with tm.assert_produces_warning(warn, match=msg):
result = df.interpolate(method=method)
if warn is not None:
# check the "use ffill instead" is equivalent
alt = df.ffill()
tm.assert_series_equal(result, alt)

expected = Series(
[1.0, 1.0, 3.0],
index=date_range("1/1/2000", periods=3, tz=tz_naive_fixture),
)
tm.assert_series_equal(result, expected)
if method == "nearest":
result = df.interpolate(method=method)
expected = Series(
[1.0, 1.0, 3.0],
index=date_range("1/1/2000", periods=3, tz=tz_naive_fixture),
)
tm.assert_series_equal(result, expected)
else:
msg = "Series cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
df.interpolate(method=method)

def test_interp_pad_datetime64tz_values(self):
# GH#27628 missing.interpolate_2d should handle datetimetz values
dti = date_range("2015-04-05", periods=3, tz="US/Central")
ser = Series(dti)
ser[1] = pd.NaT

msg = "Series.interpolate with method=pad is deprecated"
with tm.assert_produces_warning(FutureWarning, match=msg):
result = ser.interpolate(method="pad")
# check the "use ffill instead" is equivalent
alt = ser.ffill()
tm.assert_series_equal(result, alt)

expected = Series(dti)
expected[1] = expected[0]
tm.assert_series_equal(result, expected)
msg = "Series cannot interpolate with method=pad"
with pytest.raises(ValueError, match=msg):
ser.interpolate(method="pad")

def test_interp_limit_no_nans(self):
# GH 7173
Expand Down
Loading