8000 BUG: Fix exceptions when Series.interpolate's `order` parameter is missing or invalid by nmusolino · Pull Request #25246 · pandas-dev/pandas · GitHub
[go: up one dir, main page]
Skip to content

BUG: Fix exceptions when Series.interpolate's order parameter is missing or invalid #25246

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 11 commits into from
Feb 11, 2019
Merged

BUG: Fix exceptions when Series.interpolate's order parameter is missing or invalid #25246

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
de5b5d2
BUG: raise accurate exception from Series.interpolate (#24014)
nmusolino Feb 9, 2019
5bd3d4d
Actually validate `order` before use in spline
nmusolino Feb 9, 2019
c4ef8e9
Remove unnecessary check and dead code
nmusolino Feb 9, 2019
f6b8907
Clean up comparison/tests based on feedback
nmusolino Feb 10, 2019
c4f3b6d
Include invalid order value in exception
nmusolino Feb 10, 2019
2d5ec0d
Check for NaN order in spline validation
nmusolino Feb 10, 2019
097b9e4
Add whatsnew entry for bug fix
nmusolino Feb 11, 2019
becc36d
CLN: Make unit tests assert one error at a time
nmusolino Feb 11, 2019
33a0de2
CLN: break test into distinct test case
nmusolino Feb 11, 2019
24cd9f5
PEP8 fix in test module
nmusolino Feb 11, 2019
69dd362
CLN: Test fixture for interpolate methods
nmusolino Feb 11, 2019
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
Prev Previous commit
Next Next commit
Clean up comparison/tests based on feedback
  • Loading branch information
@nmusolino
nmusolino committed Feb 10, 2019
commit f6b890782b7deac0d99c319d89665d9db3139117
2 changes: 1 addition & 1 deletion pandas/core/missing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ def _interpolate_scipy_wrapper(x, y, new_x, method, fill_value=None,
new_y = terp(new_x)
elif method == 'spline':
# GH #10633, #24014
if order is None or not (0 < order):
if order is None or (order <= 0):
8000 raise ValueError("order needs to be specified and greater than 0")
terp = interpolate.UnivariateSpline(x, y, k=order, **kwargs)
new_y = terp(new_x)
Expand Down
43 changes: 19 additions & 24 deletions pandas/tests/series/test_missing.py
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -1077,18 +1077,18 @@ def test_interp_limit(self):
with pytest.raises(ValueError, match=msg):
s.interpolate(limit=limit, method=method)
Copy link
Member
@gfyoung gfyoung Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

While we can, let's do same thing here: break up this test so that we can leverage pytest parameterization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you that this test ought to be broken up and simplified.

I will try to do that tonight, but if it proves difficult, I may propose doing it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, ready for another look.


def test_interp_invalid_method(self):
@pytest.mark.parametrize("invalid_method", [None, 'nonexistent_method'])
def test_interp_invalid_method(self, invalid_method):
s = Series([1, 3, np.nan, 12, np.nan, 25])

invalid_methods = [None, 'nonexistent_method']
for method in invalid_methods:
msg = "method must be one of.*\\. Got '{}' instead".format(method)
with pytest.raises(ValueError, match=msg):
s.interpolate(method=method)
# When an invalid method and invalid limit (such as -1) are
# provided, the error message reflects the invalid method.
with pytest.raises(ValueError, match=msg):
s.interpolate(method=method, limit=-1)
msg = "method must be one of.*\\. Got '{}' instead".format(invalid_method)
with pytest.raises(ValueError, match=msg):
s.interpolate(method=invalid_method)

# When an invalid method and invalid limit (such as -1) are
# provided, the error message reflects the invalid method.
with pytest.raises(ValueError, match=msg):
s.interpolate(method=invalid_method, limit=-1)

def test_interp_limit_forward(self):
s = Series([1, 3, np.nan, np.nan, np.nan, 11])
Expand Down Expand Up @@ -1289,11 +1289,20 @@ def test_interp_limit_no_nans(self):
@td.skip_if_no_scipy
@pytest.mark.parametrize("method", ['polynomial', 'spline'])
def test_no_order(self, method):
# see GH-10633, GH-24014
s = Series([0, 1, np.nan, 3])
msg = "You must specify the order of the spline or polynomial"
with pytest.raises(ValueError, match=msg):
s.interpolate(method=method)

@td.skip_if_no_scipy
@pytest.mark.parametrize('order', [-1, -1.0, 0, 0.0])
def test_interpolate_spline_invalid_order(self, order):
s = Series([0, 1, np.nan, 3])
msg = "order needs to be specified and greater than 0"
with pytest.raises(ValueError, match=msg):
s.interpolate(method='spline', order=order)

@td.skip_if_no_scipy
def test_spline(self):
s = Series([1, 2, np.nan, 4, 5, np.nan, 7])
Expand Down Expand Up @@ -1326,20 +1335,6 @@ def test_spline_interpolation(self):
expected1 = s.interpolate(method='spline', order=1)
assert_series_equal(result1, expected1)

@td.skip_if_no_scipy
def test_spline_error(self):
# see GH-10633, GH-24014
s = pd.Series(np.arange(10) ** 2)
s[np.random.randint(0, 9, 3)] = np.nan
msg = "You must specify the order of the spline or polynomial"
with pytest.raises(ValueError, match=msg):
s.interpolate(method='spline')

msg = "order needs to be specified and greater than 0"
for invalid_order in [-1, -1., 0, 0., np.nan]:
with pytest.raises(ValueError, match=msg):
s.interpolate(method='spline', order=invalid_order)

def test_interp_timedelta64(self):
# GH 6424
df = Series([1, np.nan, 3],
Expand Down
0