Skip to content

BUG/ERR: better validation for .rolling parameters #13002

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.

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 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions doc/source/whatsnew/v0.18.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ Bug Fixes


- Bug in index coercion when falling back from ``RangeIndex`` construction (:issue:`12893`)
- Better error message in window functions when invalid argument (e.g. a float window) is passed (:issue:`12669`)

- Bug in slicing subclassed ``DataFrame`` defined to return subclassed ``Series`` may return normal ``Series`` (:issue:`11559`)

Expand Down Expand Up @@ -511,6 +512,7 @@ Bug Fixes
- Bug in ``crosstab`` when ``margins=True`` and ``dropna=False`` which raised (:issue:`12642`)

- Bug in ``Series.name`` when ``name`` attribute can be a hashable type (:issue:`12610`)

- Bug in ``.describe()`` resets categorical columns information (:issue:`11558`)
- Bug where ``loffset`` argument was not applied when calling ``resample().count()`` on a timeseries (:issue:`12725`)
- ``pd.read_excel()`` now accepts column names associated with keyword argument ``names``(:issue:`12870`)
87 changes: 58 additions & 29 deletions pandas/core/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,20 @@ def __init__(self, obj, window=None, min_periods=None, freq=None,
self.freq = freq
self.center = center
self.win_type = win_type
self.axis = axis
self.axis = obj._get_axis_number(axis) if axis is not None else None
self.validate()

@property
def _constructor(self):
return Window

def validate(self):
if self.center is not None and not com.is_bool(self.center):
raise ValueError("center must be a boolean")
if self.min_periods is not None and not \
com.is_integer(self.min_periods):
raise ValueError("min_periods must be an integer")

def _convert_freq(self, how=None):
""" resample according to the how, return a new object """

Expand Down Expand Up @@ -305,24 +313,62 @@ class Window(_Window):
* ``slepian`` (needs width).
"""

def _prep_window(self, **kwargs):
""" provide validation for our window type, return the window """
window = self._get_window()
def validate(self):
super(Window, self).validate()

window = self.window
if isinstance(window, (list, tuple, np.ndarray)):
return com._asarray_tuplesafe(window).astype(float)
pass
elif com.is_integer(window):
try:
import scipy.signal as sig
except ImportError:
raise ImportError('Please install scipy to generate window '
'weight')

if not isinstance(self.win_type, compat.string_types):
raise ValueError('Invalid win_type {0}'.format(self.win_type))
if getattr(sig, self.win_type, None) is None:
raise ValueError('Invalid win_type {0}'.format(self.win_type))
else:
raise ValueError('Invalid window {0}'.format(window))

def _prep_window(self, **kwargs):
"""
provide validation for our window type, return the window
we have already been validated
"""

window = self._get_window()
if isinstance(window, (list, tuple, np.ndarray)):
return com._asarray_tuplesafe(window).astype(float)
elif com.is_integer(window):
import scipy.signal as sig

# the below may pop from kwargs
def _validate_win_type(win_type, kwargs):
arg_map = {'kaiser': ['beta'],
'gaussian': ['std'],
'general_gaussian': ['power', 'width'],
'slepian': ['width']}
if win_type in arg_map:
return tuple([win_type] + _pop_args(win_type,
arg_map[win_type],
kwargs))
return win_type

def _pop_args(win_type, arg_names, kwargs):
msg = '%s window requires %%s' % win_type
all_args = []
for n in arg_names:
if n not in kwargs:
raise ValueError(msg % n)
all_args.append(kwargs.pop(n))
return all_args

win_type = _validate_win_type(self.win_type, kwargs)
return sig.get_window(win_type, window).astype(float)

raise ValueError('Invalid window %s' % str(window))

def _apply_window(self, mean=True, how=None, **kwargs):
"""
Applies a moving window of type ``window_type`` on the data.
Expand Down Expand Up @@ -791,6 +837,11 @@ class Rolling(_Rolling_and_Expanding):
of :meth:`~pandas.Series.resample` (i.e. using the `mean`).
"""

def validate(self):
super(Rolling, self).validate()
if not com.is_integer(self.window):
raise ValueError("window must be an integer")

@Substitution(name='rolling')
@Appender(SelectionMixin._see_also_template)
@Appender(SelectionMixin._agg_doc)
Expand Down Expand Up @@ -1459,28 +1510,6 @@ def _prep_binary(arg1, arg2):
return X, Y


def _validate_win_type(win_type, kwargs):
# may pop from kwargs
arg_map = {'kaiser': ['beta'],
'gaussian': ['std'],
'general_gaussian': ['power', 'width'],
'slepian': ['width']}
if win_type in arg_map:
return tuple([win_type] + _pop_args(win_type, arg_map[win_type],
kwargs))
return win_type


def _pop_args(win_type, arg_names, kwargs):
msg = '%s window requires %%s' % win_type
all_args = []
for n in arg_names:
if n not in kwargs:
raise ValueError(msg % n)
all_args.append(kwargs.pop(n))
return all_args


# Top-level exports


Expand Down
84 changes: 84 additions & 0 deletions pandas/tests/test_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,90 @@ def test_how_compat(self):
assert_series_equal(result, expected)


class TestWindow(Base):

def setUp(self):
self._create_data()

def test_constructor(self):
# GH 12669
tm._skip_if_no_scipy()

for o in [self.series, self.frame]:
c = o.rolling

# valid
c(win_type='boxcar', window=2, min_periods=1)
c(win_type='boxcar', window=2, min_periods=1, center=True)
c(win_type='boxcar', window=2, min_periods=1, center=False)

for wt in ['boxcar', 'triang', 'blackman', 'hamming', 'bartlett',
'bohman', 'blackmanharris', 'nuttall', 'barthann']:
c(win_type=wt, window=2)

# not valid
for w in [2., 'foo', np.array([2])]:
with self.assertRaises(ValueError):
c(win_type='boxcar', window=2, min_periods=w)
with self.assertRaises(ValueError):
c(win_type='boxcar', window=2, min_periods=1, center=w)

for wt in ['foobar', 1]:
with self.assertRaises(ValueError):
c(win_type=wt, window=2)


class TestRolling(Base):

def setUp(self):
self._create_data()

def test_constructor(self):
# GH 12669

for o in [self.series, self.frame]:
c = o.rolling

# valid
c(window=2)
c(window=2, min_periods=1)
c(window=2, min_periods=1, center=True)
c(window=2, min_periods=1, center=False)

# not valid
for w in [2., 'foo', np.array([2])]:
with self.assertRaises(ValueError):
c(window=w)
with self.assertRaises(ValueError):
c(window=2, min_periods=w)
with self.assertRaises(ValueError):
c(window=2, min_periods=1, center=w)


class TestExpanding(Base):

def setUp(self):
self._create_data()

def test_constructor(self):
# GH 12669

for o in [self.series, self.frame]:
c = o.expanding

# valid
c(min_periods=1)
c(min_periods=1, center=True)
c(min_periods=1, center=False)

# not valid
for w in [2., 'foo', np.array([2])]:
with self.assertRaises(ValueError):
c(min_periods=w)
with self.assertRaises(ValueError):
c(min_periods=1, center=w)


class TestDeprecations(Base):
""" test that we are catching deprecation warnings """

Expand Down