BUG/ERR: better validation for .rolling parameters

closes #12669

Author: Jeff Reback <[email protected]>

Closes #13002 from jreback/window and squashes the following commits:

985f324 [Jeff Reback] BUG: better validation for .rolling parameters

Author: jreback

doc/source/whatsnew/v0.18.1.txt

@@ -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`)
 
@@ -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`)

pandas/core/window.py

@@ -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 """
 
@@ -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.
@@ -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)
@@ -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
 
 

pandas/tests/test_window.py

@@ -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 """