Prevent passing invalid kwds to DateOffset constructors

doc/source/whatsnew/v0.22.0.txt

@@ -47,7 +47,7 @@ Other API Changes
 - :class:`CacheableOffset` and :class:`WeekDay` are no longer available in the ``pandas.tseries.offsets`` module (:issue:`17830`)
 - `tseries.frequencies.get_freq_group()` and `tseries.frequencies.DAYS` are removed from the public API (:issue:`18034`)
 - :func:`Series.truncate` and :func:`DataFrame.truncate` will raise a ``ValueError`` if the index is not sorted instead of an unhelpful ``KeyError`` (:issue:`17935`)
-
+- Restricted ``DateOffset`` keyword arguments. Previously, ``DateOffset`` subclasses allowed arbitrary keyword arguments which could lead to unexpected behavior. Now, only valid arguments will be accepted. (:issue:`17176`, :issue:`18226`).
 
 .. _whatsnew_0220.deprecations:
 

pandas/_libs/tslibs/offsets.pyx

@@ -255,7 +255,7 @@ def _validate_business_time(t_input):
 # ---------------------------------------------------------------------
 # Constructor Helpers
 
-_rd_kwds = set([
+relativedelta_kwds = set([
     'years', 'months', 'weeks', 'days',
     'year', 'month', 'week', 'day', 'weekday',
     'hour', 'minute', 'second', 'microsecond',
@@ -394,6 +394,15 @@ class _BaseOffset(object):
         out = '<%s' % n_str + className + plural + self._repr_attrs() + '>'
         return out
 
+    def _validate_n(self, n):
+        try:
+            nint = int(n)
+        except (ValueError, TypeError):
+            raise ValueError('`n` argument must be an integer')
+        if n != nint:
+            raise ValueError('`n` argument must be an integer')
+        return nint
+
 
 class BaseOffset(_BaseOffset):
     # Here we add __rfoo__ methods that don't play well with cdef classes

pandas/tests/tseries/offsets/conftest.py

@@ -7,6 +7,19 @@ def offset_types(request):
     return request.param
 
 
+@pytest.fixture(params=[getattr(offsets, o) for o in offsets.__all__ if
+                        issubclass(getattr(offsets, o), offsets.MonthOffset)
+                        and o != 'MonthOffset'])
+def month_classes(request):
+    return request.param
+
+
+@pytest.fixture(params=[getattr(offsets, o) for o in offsets.__all__ if
+                        issubclass(getattr(offsets, o), offsets.Tick)])
+def tick_classes(request):
+    return request.param
+
+
 @pytest.fixture(params=[None, 'UTC', 'Asia/Tokyo', 'US/Eastern',
                         'dateutil/Asia/Tokyo', 'dateutil/US/Pacific'])
 def tz(request):

pandas/tests/tseries/offsets/test_offsets.py

@@ -17,7 +17,8 @@
                                         get_offset, get_standard_freq)
 from pandas.core.indexes.datetimes import (
     _to_m8, DatetimeIndex, _daterange_cache)
-from pandas._libs.tslibs.offsets import WeekDay, CacheableOffset
+import pandas._libs.tslibs.offsets as liboffsets
+from pandas._libs.tslibs.offsets import WeekDay, CacheableOffset, relativedelta_kwds
 from pandas.tseries.offsets import (BDay, CDay, BQuarterEnd, BMonthEnd,
                                     BusinessHour, WeekOfMonth, CBMonthEnd,
                                     CustomBusinessHour,
@@ -4680,3 +4681,29 @@ def test_all_offset_classes(self, tup):
         first = Timestamp(test_values[0], tz='US/Eastern') + offset()
         second = Timestamp(test_values[1], tz='US/Eastern')
         assert first == second
+
+
+# ---------------------------------------------------------------------
+
+
+@pytest.mark.parametrize('kwd', sorted(list(liboffsets.relativedelta_kwds)))
+def test_valid_month_attributes(kwd, month_classes):
+    # GH#18226
+    cls = month_classes
+    # check that we cannot create e.g. MonthEnd(weeks=3)
+    with pytest.raises(TypeError):
+        cls(**{kwd: 3})
+
+
+@pytest.mark.parametrize('kwd', sorted(list(liboffsets.relativedelta_kwds)))
+def test_valid_tick_attributes(kwd, tick_classes):
+    # GH#18226
+    cls = tick_classes
+    # check that we cannot create e.g. Hour(weeks=3)
+    with pytest.raises(TypeError):
+        cls(**{kwd: 3})
+
+
+def test_validate_n_error():
+    with pytest.raises(ValueError):
+        DateOffset(n='Doh!')

pandas/tseries/offsets.py

@@ -1,5 +1,8 @@
 # -*- coding: utf-8 -*-
 from datetime import date, datetime, timedelta
+import functools
+import operator
+
 from pandas.compat import range
 from pandas import compat
 import numpy as np
@@ -28,8 +31,6 @@
     BeginMixin, EndMixin,
     BaseOffset)
 
-import functools
-import operator
 
 __all__ = ['Day', 'BusinessDay', 'BDay', 'CustomBusinessDay', 'CDay',
            'CBMonthEnd', 'CBMonthBegin',
@@ -166,7 +167,7 @@ def __add__(date):
     normalize = False
 
     def __init__(self, n=1, normalize=False, **kwds):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.kwds = kwds
 
@@ -473,7 +474,7 @@ class BusinessDay(BusinessMixin, SingleConstructorOffset):
     _adjust_dst = True
 
     def __init__(self, n=1, normalize=False, offset=timedelta(0)):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.kwds = {'offset': offset}
         self._offset = offset
@@ -782,7 +783,7 @@ class BusinessHour(BusinessHourMixin, SingleConstructorOffset):
 
     def __init__(self, n=1, normalize=False, start='09:00',
                  end='17:00', offset=timedelta(0)):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         super(BusinessHour, self).__init__(start=start, end=end, offset=offset)
 
@@ -819,7 +820,7 @@ class CustomBusinessDay(BusinessDay):
 
     def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri',
                  holidays=None, calendar=None, offset=timedelta(0)):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self._offset = offset
         self.kwds = {}
@@ -887,7 +888,7 @@ class CustomBusinessHour(BusinessHourMixin, SingleConstructorOffset):
     def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri',
                  holidays=None, calendar=None,
                  start='09:00', end='17:00', offset=timedelta(0)):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         super(CustomBusinessHour, self).__init__(start=start,
                                                  end=end, offset=offset)
@@ -915,6 +916,13 @@ def next_bday(self):
 class MonthOffset(SingleConstructorOffset):
     _adjust_dst = True
 
+    def __init__(self, n=1, normalize=False):
+        self.n = self._validate_n(n)
+        self.normalize = normalize
+        self._offset = timedelta(1)
+        self._use_relativedelta = False
+        self.kwds = {}
+
     @property
     def name(self):
         if self.isAnchored:
@@ -989,7 +997,8 @@ def __init__(self, n=1, normalize=False, day_of_month=None):
             msg = 'day_of_month must be {min}<=day_of_month<=27, got {day}'
             raise ValueError(msg.format(min=self._min_day_of_month,
                                         day=self.day_of_month))
-        self.n = int(n)
+
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.kwds = {'day_of_month': self.day_of_month}
 
@@ -1242,7 +1251,7 @@ class CustomBusinessMonthEnd(BusinessMixin, MonthOffset):
 
     def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri',
                  holidays=None, calendar=None, offset=timedelta(0)):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self._offset = offset
         self.kwds = {}
@@ -1313,7 +1322,7 @@ class CustomBusinessMonthBegin(BusinessMixin, MonthOffset):
 
     def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri',
                  holidays=None, calendar=None, offset=timedelta(0)):
-        self.n = int(n)
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self._offset = offset
         self.kwds = {}
@@ -1377,7 +1386,7 @@ class Week(EndMixin, DateOffset):
     _prefix = 'W'
 
     def __init__(self, n=1, normalize=False, weekday=None):
-        self.n = n
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.weekday = weekday
 
@@ -1456,7 +1465,7 @@ class WeekOfMonth(DateOffset):
     _adjust_dst = True
 
     def __init__(self, n=1, normalize=False, week=None, weekday=None):
-        self.n = n
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.weekday = weekday
         self.week = week
@@ -1541,7 +1550,7 @@ class LastWeekOfMonth(DateOffset):
     _prefix = 'LWOM'
 
     def __init__(self, n=1, normalize=False, weekday=None):
-        self.n = n
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.weekday = weekday
 
@@ -1604,13 +1613,15 @@ class QuarterOffset(DateOffset):
     #       point
 
     def __init__(self, n=1, normalize=False, startingMonth=None):
-        self.n = n
+        self.n = self._validate_n(n)
         self.normalize = normalize
         if startingMonth is None:
             startingMonth = self._default_startingMonth
         self.startingMonth = startingMonth
 
         self.kwds = {'startingMonth': startingMonth}
+        self._offset = timedelta(1)
+        self._use_relativedelta = False
 
     def isAnchored(self):
         return (self.n == 1 and self.startingMonth is not None)
@@ -1946,7 +1957,7 @@ class FY5253(DateOffset):
 
     def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1,
                  variation="nearest"):
-        self.n = n
+        self.n = self._validate_n(n)
         self.normalize = normalize
         self.startingMonth = startingMonth
         self.weekday = weekday
@@ -2196,7 +2207,7 @@ class FY5253Quarter(DateOffset):
 
     def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1,
                  qtr_with_extra_week=1, variation="nearest"):
-        self.n = n
+        self.n = self._validate_n(n)
         self.normalize = normalize
 
         self.weekday = weekday
@@ -2323,6 +2334,13 @@ class Easter(DateOffset):
     """
     _adjust_dst = True
 
+    def __init__(self, n=1, normalize=False):
+        self.n = self._validate_n(n)
+        self.normalize = normalize
+        self._offset = timedelta(1)
+        self._use_relativedelta = False
+        self.kwds = {}
+
     @apply_wraps
     def apply(self, other):
         currentEaster = easter(other.year)
@@ -2367,6 +2385,14 @@ class Tick(SingleConstructorOffset):
     _inc = Timedelta(microseconds=1000)
     _prefix = 'undefined'
 
+    def __init__(self, n=1, normalize=False):
+        # TODO: do Tick classes with normalize=True make sense?
+        self.n = self._validate_n(n)
+        self.normalize = normalize
+        self._offset = timedelta(1)
+        self._use_relativedelta = False
+        self.kwds = {}
+
     __gt__ = _tick_comp(operator.gt)
     __ge__ = _tick_comp(operator.ge)
     __lt__ = _tick_comp(operator.lt)
@@ -2425,6 +2451,7 @@ def delta(self):
     def nanos(self):
         return delta_to_nanoseconds(self.delta)
 
+    # TODO: Should Tick have its own apply_index?
     def apply(self, other):
         # Timestamp can handle tz and nano sec, thus no need to use apply_wraps
         if isinstance(other, Timestamp):