Skip to content

compatibility with scipy 0.19 #15689

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

Closed
wants to merge 3 commits into from
Closed

compatibility with scipy 0.19 #15689

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca09705
get symmetric window
zym1010 Mar 14, 2017
9ed7524
fix interpolation related issue with scipy 0.19
zym1010 Mar 14, 2017
3cc6528
doc and PEP8
zym1010 Mar 15, 2017
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
4 changes: 3 additions & 1 deletion pandas/core/window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,9 @@ def _pop_args(win_type, arg_names, kwargs):
return all_args

win_type = _validate_win_type(self.win_type, kwargs)
return sig.get_window(win_type, window).astype(float)
# GH #15662.
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need these comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright. so no comment at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea you can take it out, unless passing the arg is meaningful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean by "meaningful"?

Copy link
Contributor

Choose a reason for hiding this comment

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

if it were non-obvious what this parameter means / did (but passing it as False). So on 2nd thought if you can provide a 1-line comment on the meaning would be good.

# fftbins should be False; previously it was a mistake on pandas' side.
return sig.get_window(win_type, window, fftbins=False).astype(float)

def _apply_window(self, mean=True, how=None, **kwargs):
"""
Expand Down
23 changes: 14 additions & 9 deletions pandas/tests/frame/test_missing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ def test_interp_nan_idx(self):
df.interpolate(method='values')

def test_interp_various(self):
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

df = DataFrame({'A': [1, 2, np.nan, 4, 5, np.nan, 7],
'C': [1, 2, 3, 5, 8, 13, 21]})
Expand All @@ -561,8 +561,13 @@ def test_interp_various(self):
assert_frame_equal(result, expected)

result = df.interpolate(method='cubic')
expected.A.loc[3] = 2.81621174
expected.A.loc[13] = 5.64146581
import scipy
Copy link
Contributor

Choose a reason for hiding this comment

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

you can define a variable at the very top of the file

try:
    import scipy
    _is_scipy_ge_0190 = scipy.__version__ >= LooseVersion(....)
exept:
   _is_scipy_ge_0190 = False

if scipy.__version__ >= LooseVersion('0.19.0'):
expected.A.loc[3] = 2.81547781
Copy link
Contributor

Choose a reason for hiding this comment

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

also can you add a comment here (1-liner), on what qualitatively changed (adding the link is ok too).

imagine you are future reader and you have no idea why this was done....

expected.A.loc[13] = 5.52964175
else:
expected.A.loc[3] = 2.81621174
expected.A.loc[13] = 5.64146581
assert_frame_equal(result, expected)

result = df.interpolate(method='nearest')
Expand All @@ -571,8 +576,12 @@ def test_interp_various(self):
assert_frame_equal(result, expected, check_dtype=False)

result = df.interpolate(method='quadratic')
expected.A.loc[3] = 2.82533638
expected.A.loc[13] = 6.02817974
if scipy.__version__ >= LooseVersion('0.19.0'):
expected.A.loc[3] = 2.82150771
expected.A.loc[13] = 6.12648668
else:
expected.A.loc[3] = 2.82533638
expected.A.loc[13] = 6.02817974
Copy link

@ev-br ev-br Mar 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Generally, I'd recommend to avoid hard-coding the floating-point numbers like this: these are implementation details really. The values at round numbers should not change though. Or, at least, I'd use a relatively loose tolerance to smooth over these implementation details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ev-br point taken. Yet I think as this is the way pandas test suites work for a very long time, it should not break arbitrarily. These computation results should be pretty stable, across different architectures, etc., for a given scipy version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ev-br main issue is that, somehow quadratic and cubic interpolation results revert back to pre 0.19 behavior.

assert_frame_equal(result, expected)

result = df.interpolate(method='slinear')
Expand All @@ -585,10 +594,6 @@ def test_interp_various(self):
expected.A.loc[13] = 5
assert_frame_equal(result, expected, check_dtype=False)

result = df.interpolate(method='quadratic')
expected.A.loc[3] = 2.82533638
expected.A.loc[13] = 6.02817974
assert_frame_equal(result, expected)

def test_interp_alt_scipy(self):
tm._skip_if_no_scipy()
Expand Down
9 changes: 7 additions & 2 deletions pandas/tests/series/test_missing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytz
from datetime import timedelta, datetime

from distutils.version import LooseVersion
from numpy import nan
import numpy as np
import pandas as pd
Expand Down Expand Up @@ -827,7 +828,7 @@ def test_interp_quad(self):
assert_series_equal(result, expected)

def test_interp_scipy_basic(self):
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

s = Series([1, 3, np.nan, 12, np.nan, 25])
# slinear
Expand All @@ -852,7 +853,11 @@ def test_interp_scipy_basic(self):
result = s.interpolate(method='zero', downcast='infer')
assert_series_equal(result, expected)
# quadratic
expected = Series([1, 3., 6.769231, 12., 18.230769, 25.])
import scipy
if scipy.__version__ >= LooseVersion('0.19.0'):
expected = Series([1, 3., 6.823529, 12., 18.058824, 25.])
else:
expected = Series([1, 3., 6.769231, 12., 18.230769, 25.])
result = s.interpolate(method='quadratic')
assert_series_equal(result, expected)

Expand Down
10 changes: 5 additions & 5 deletions pandas/tests/test_window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ def test_cmov_window_na_min_periods(self):

def test_cmov_window_regular(self):
# GH 8238
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

win_types = ['triang', 'blackman', 'hamming', 'bartlett', 'bohman',
'blackmanharris', 'nuttall', 'barthann']
Expand Down Expand Up @@ -938,7 +938,7 @@ def test_cmov_window_regular(self):

def test_cmov_window_regular_linear_range(self):
# GH 8238
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

win_types = ['triang', 'blackman', 'hamming', 'bartlett', 'bohman',
'blackmanharris', 'nuttall', 'barthann']
Expand All @@ -955,7 +955,7 @@ def test_cmov_window_regular_linear_range(self):

def test_cmov_window_regular_missing_data(self):
# GH 8238
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

win_types = ['triang', 'blackman', 'hamming', 'bartlett', 'bohman',
'blackmanharris', 'nuttall', 'barthann']
Expand Down Expand Up @@ -988,7 +988,7 @@ def test_cmov_window_regular_missing_data(self):

def test_cmov_window_special(self):
# GH 8238
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

win_types = ['kaiser', 'gaussian', 'general_gaussian', 'slepian']
kwds = [{'beta': 1.}, {'std': 1.}, {'power': 2.,
Expand All @@ -1015,7 +1015,7 @@ def test_cmov_window_special(self):

def test_cmov_window_special_linear_range(self):
# GH 8238
tm.skip_if_no_package('scipy', max_version='0.19.0')
tm._skip_if_no_scipy()

win_types = ['kaiser', 'gaussian', 'general_gaussian', 'slepian']
kwds = [{'beta': 1.}, {'std': 1.}, {'power': 2.,
Expand Down