Fix offset __inits__, apply_index dtypes (#19142)

Author: jbrockmendel

doc/source/whatsnew/v0.23.0.txt

@@ -381,7 +381,9 @@ Conversion
 - Fixed bug where comparing :class:`DatetimeIndex` failed to raise ``TypeError`` when attempting to compare timezone-aware and timezone-naive datetimelike objects (:issue:`18162`)
 - Bug in :class:`DatetimeIndex` where the repr was not showing high-precision time values at the end of a day (e.g., 23:59:59.999999999) (:issue:`19030`)
 - Bug where dividing a scalar timedelta-like object with :class:`TimedeltaIndex` performed the reciprocal operation (:issue:`19125`)
+- Bug in :class:`WeekOfMonth` and :class:`LastWeekOfMonth` where default keyword arguments for constructor raised ``ValueError`` (:issue:`19142`)
 - Bug in localization of a naive, datetime string in a ``Series`` constructor with a ``datetime64[ns, tz]`` dtype (:issue:`174151`)
+- :func:`Timestamp.replace` will now handle Daylight Savings transitions gracefully (:issue:`18319`)
 
 Indexing
 ^^^^^^^^
@@ -473,4 +475,3 @@ Other
 ^^^^^
 
 - Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`)
-- :func:`Timestamp.replace` will now handle Daylight Savings transitions gracefully (:issue:`18319`)

pandas/_libs/tslibs/offsets.pyx

@@ -290,27 +290,6 @@ class CacheableOffset(object):
     _cacheable = True
 
 
-class EndMixin(object):
-    # helper for vectorized offsets
-
-    def _end_apply_index(self, i, freq):
-        """Offsets index to end of Period frequency"""
-
-        off = i.to_perioddelta('D')
-
-        base, mult = get_freq_code(freq)
-        base_period = i.to_period(base)
-        if self.n > 0:
-            # when adding, dates on end roll to next
-            roll = np.where(base_period.to_timestamp(how='end') == i - off,
-                            self.n, self.n - 1)
-        else:
-            roll = self.n
-
-        base = (base_period + roll).to_timestamp(how='end')
-        return base + off
-
-
 # ---------------------------------------------------------------------
 # Base Classes
 
@@ -675,11 +654,8 @@ def shift_months(int64_t[:] dtindex, int months, object day=None):
                 months_to_roll = months
                 compare_day = get_firstbday(dts.year, dts.month)
 
-                if months_to_roll > 0 and dts.day < compare_day:
-                    months_to_roll -= 1
-                elif months_to_roll <= 0 and dts.day > compare_day:
-                    # as if rolled forward already
-                    months_to_roll += 1
+                months_to_roll = roll_convention(dts.day, months_to_roll,
+                                                 compare_day)
 
                 dts.year = year_add_months(dts, months_to_roll)
                 dts.month = month_add_months(dts, months_to_roll)
@@ -698,11 +674,8 @@ def shift_months(int64_t[:] dtindex, int months, object day=None):
                 months_to_roll = months
                 compare_day = get_lastbday(dts.year, dts.month)
 
-                if months_to_roll > 0 and dts.day < compare_day:
-                    months_to_roll -= 1
-                elif months_to_roll <= 0 and dts.day > compare_day:
-                    # as if rolled forward already
-                    months_to_roll += 1
+                months_to_roll = roll_convention(dts.day, months_to_roll,
+                                                 compare_day)
 
                 dts.year = year_add_months(dts, months_to_roll)
                 dts.month = month_add_months(dts, months_to_roll)
@@ -823,7 +796,7 @@ cpdef int get_day_of_month(datetime other, day_opt) except? -1:
         raise ValueError(day_opt)
 
 
-cpdef int roll_convention(int other, int n, int compare):
+cpdef int roll_convention(int other, int n, int compare) nogil:
     """
     Possibly increment or decrement the number of periods to shift
     based on rollforward/rollbackward conventions.

pandas/tests/indexes/datetimes/test_arithmetic.py

@@ -495,7 +495,7 @@ def test_dt64_with_DateOffsets_relativedelta(klass, assert_func):
         assert_func(klass([x - op for x in vec]), vec - op)
 
 
-@pytest.mark.parametrize('cls_name', [
+@pytest.mark.parametrize('cls_and_kwargs', [
     'YearBegin', ('YearBegin', {'month': 5}),
     'YearEnd', ('YearEnd', {'month': 5}),
     'MonthBegin', 'MonthEnd',
@@ -518,7 +518,7 @@ def test_dt64_with_DateOffsets_relativedelta(klass, assert_func):
 @pytest.mark.parametrize('klass,assert_func', [
     (Series, tm.assert_series_equal),
     (DatetimeIndex, tm.assert_index_equal)])
-def test_dt64_with_DateOffsets(klass, assert_func, normalize, cls_name):
+def test_dt64_with_DateOffsets(klass, assert_func, normalize, cls_and_kwargs):
     # GH#10699
     # assert these are equal on a piecewise basis
     vec = klass([Timestamp('2000-01-05 00:15:00'),
@@ -530,11 +530,12 @@ def test_dt64_with_DateOffsets(klass, assert_func, normalize, cls_name):
                  Timestamp('2000-05-15'),
                  Timestamp('2001-06-15')])
 
-    if isinstance(cls_name, tuple):
+    if isinstance(cls_and_kwargs, tuple):
         # If cls_name param is a tuple, then 2nd entry is kwargs for
         # the offset constructor
-        cls_name, kwargs = cls_name
+        cls_name, kwargs = cls_and_kwargs
     else:
+        cls_name = cls_and_kwargs
         kwargs = {}
 
     offset_cls = getattr(pd.offsets, cls_name)

pandas/tests/tseries/offsets/test_offsets.py

@@ -3087,6 +3087,13 @@ def test_get_offset_day_error():
         DateOffset()._get_offset_day(datetime.now())
 
 
+def test_valid_default_arguments(offset_types):
+    # GH#19142 check that the calling the constructors without passing
+    # any keyword arguments produce valid offsets
+    cls = offset_types
+    cls()
+
+
 @pytest.mark.parametrize('kwd', sorted(list(liboffsets.relativedelta_kwds)))
 def test_valid_month_attributes(kwd, month_classes):
     # GH#18226

pandas/tseries/offsets.py

@@ -16,7 +16,7 @@
 from pandas._libs import tslib, Timestamp, OutOfBoundsDatetime, Timedelta
 from pandas.util._decorators import cache_readonly
 
-from pandas._libs.tslibs import ccalendar
+from pandas._libs.tslibs import ccalendar, frequencies as libfrequencies
 from pandas._libs.tslibs.timedeltas import delta_to_nanoseconds
 import pandas._libs.tslibs.offsets as liboffsets
 from pandas._libs.tslibs.offsets import (
@@ -27,7 +27,6 @@
     apply_index_wraps,
     roll_yearday,
     shift_month,
-    EndMixin,
     BaseOffset)
 
 
@@ -1233,7 +1232,19 @@ def _get_roll(self, i, before_day_of_month, after_day_of_month):
         return roll
 
     def _apply_index_days(self, i, roll):
-        i += (roll % 2) * Timedelta(days=self.day_of_month).value
+        """Add days portion of offset to DatetimeIndex i
+
+        Parameters
+        ----------
+        i : DatetimeIndex
+        roll : ndarray[int64_t]
+
+        Returns
+        -------
+        result : DatetimeIndex
+        """
+        nanos = (roll % 2) * Timedelta(days=self.day_of_month).value
+        i += nanos.astype('timedelta64[ns]')
         return i + Timedelta(days=-1)
 
 
@@ -1278,13 +1289,25 @@ def _get_roll(self, i, before_day_of_month, after_day_of_month):
         return roll
 
     def _apply_index_days(self, i, roll):
-        return i + (roll % 2) * Timedelta(days=self.day_of_month - 1).value
+        """Add days portion of offset to DatetimeIndex i
+
+        Parameters
+        ----------
+        i : DatetimeIndex
+        roll : ndarray[int64_t]
+
+        Returns
+        -------
+        result : DatetimeIndex
+        """
+        nanos = (roll % 2) * Timedelta(days=self.day_of_month - 1).value
+        return i + nanos.astype('timedelta64[ns]')
 
 
 # ---------------------------------------------------------------------
 # Week-Based Offset Classes
 
-class Week(EndMixin, DateOffset):
+class Week(DateOffset):
     """
     Weekly offset
 
@@ -1332,7 +1355,34 @@ def apply_index(self, i):
             return ((i.to_period('W') + self.n).to_timestamp() +
                     i.to_perioddelta('W'))
         else:
-            return self._end_apply_index(i, self.freqstr)
+            return self._end_apply_index(i)
+
+    def _end_apply_index(self, dtindex):
+        """Add self to the given DatetimeIndex, specialized for case where
+        self.weekday is non-null.
+
+        Parameters
+        ----------
+        dtindex : DatetimeIndex
+
+        Returns
+        -------
+        result : DatetimeIndex
+        """
+        off = dtindex.to_perioddelta('D')
+
+        base, mult = libfrequencies.get_freq_code(self.freqstr)
+        base_period = dtindex.to_period(base)
+        if self.n > 0:
+            # when adding, dates on end roll to next
+            normed = dtindex - off
+            roll = np.where(base_period.to_timestamp(how='end') == normed,
+                            self.n, self.n - 1)
+        else:
+            roll = self.n
+
+        base = (base_period + roll).to_timestamp(how='end')
+        return base + off
 
     def onOffset(self, dt):
         if self.normalize and not _is_normalized(dt):
@@ -1387,9 +1437,9 @@ class WeekOfMonth(_WeekOfMonthMixin, DateOffset):
     Parameters
     ----------
     n : int
-    week : {0, 1, 2, 3, ...}, default None
+    week : {0, 1, 2, 3, ...}, default 0
         0 is 1st week of month, 1 2nd week, etc.
-    weekday : {0, 1, ..., 6}, default None
+    weekday : {0, 1, ..., 6}, default 0
         0: Mondays
         1: Tuesdays
         2: Wednesdays
@@ -1401,7 +1451,7 @@ class WeekOfMonth(_WeekOfMonthMixin, DateOffset):
     _prefix = 'WOM'
     _adjust_dst = True
 
-    def __init__(self, n=1, normalize=False, week=None, weekday=None):
+    def __init__(self, n=1, normalize=False, week=0, weekday=0):
         self.n = self._validate_n(n)
         self.normalize = normalize
         self.weekday = weekday
@@ -1464,7 +1514,7 @@ class LastWeekOfMonth(_WeekOfMonthMixin, DateOffset):
     Parameters
     ----------
     n : int, default 1
-    weekday : {0, 1, ..., 6}, default None
+    weekday : {0, 1, ..., 6}, default 0
         0: Mondays
         1: Tuesdays
         2: Wednesdays
@@ -1477,7 +1527,7 @@ class LastWeekOfMonth(_WeekOfMonthMixin, DateOffset):
     _prefix = 'LWOM'
     _adjust_dst = True
 
-    def __init__(self, n=1, normalize=False, weekday=None):
+    def __init__(self, n=1, normalize=False, weekday=0):
         self.n = self._validate_n(n)
         self.normalize = normalize
         self.weekday = weekday