From b53f0f9c4829942a273add69468a7dc03a23751e Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 16 Nov 2017 10:37:26 -0800 Subject: [PATCH 1/7] Define _day_opt attributes to unify apply and onOffset implementations --- pandas/_libs/tslibs/offsets.pyx | 25 +++++- pandas/tseries/offsets.py | 137 +++++++++++++++++++------------- 2 files changed, 102 insertions(+), 60 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 2b896bcc930a7..507a8504b6562 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -394,6 +394,10 @@ class _BaseOffset(object): out = '<%s' % n_str + className + plural + self._repr_attrs() + '>' return out + def _get_offset_day(self, datetime other): + # subclass must implement `_day_opt` + return get_day_of_month(other, self._day_opt) + class BaseOffset(_BaseOffset): # Here we add __rfoo__ methods that don't play well with cdef classes @@ -468,7 +472,7 @@ cpdef datetime shift_month(datetime stamp, int months, object day_opt=None): return stamp.replace(year=year, month=month, day=day) -cdef int get_day_of_month(datetime other, day_opt) except? -1: +cpdef int get_day_of_month(datetime other, day_opt) except? -1: """ Find the day in `other`'s month that satisfies a DateOffset's onOffset policy, as described by the `day_opt` argument. @@ -493,11 +497,26 @@ cdef int get_day_of_month(datetime other, day_opt) except? -1: 30 """ + cdef: + int wkday, days_in_month + if day_opt == 'start': return 1 - elif day_opt == 'end': - return monthrange(other.year, other.month)[1] + + wkday, days_in_month = monthrange(other.year, other.month) + if day_opt == 'end': + return days_in_month + elif day_opt == 'business_start': + # first business day of month + return get_firstbday(wkday, days_in_month) + elif day_opt == 'business_end': + # last business day of month + return get_lastbday(wkday, days_in_month) + elif is_integer_object(day_opt): + day = min(day_opt, days_in_month) else: + # Note: unlike `shift_month`, this get_day_of_month does not + # allow day_opt = None raise ValueError(day_opt) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 615d703f66932..d66903ec08edb 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -15,6 +15,7 @@ from pandas.util._decorators import cache_readonly from pandas._libs.tslibs.timedeltas import delta_to_nanoseconds +import pandas._libs.tslibs.offsets as liboffsets from pandas._libs.tslibs.offsets import ( ApplyTypeError, as_datetime, _is_normalized, @@ -912,6 +913,10 @@ def next_bday(self): calendar=self.calendar) +# --------------------------------------------------------------------- +# Month-Based Offset Classes + + class MonthOffset(SingleConstructorOffset): _adjust_dst = True @@ -927,52 +932,54 @@ def name(self): class MonthEnd(MonthOffset): """DateOffset of one month end""" _prefix = 'M' + _day_opt = 'end' @apply_wraps def apply(self, other): n = self.n - _, days_in_month = tslib.monthrange(other.year, other.month) - if other.day != days_in_month: - other = shift_month(other, -1, 'end') + compare_day = self._get_offset_day(other) + if other.day < compare_day: + other = shift_month(other, -1, self._day_opt) if n <= 0: n = n + 1 - other = shift_month(other, n, 'end') + other = shift_month(other, n, self._day_opt) return other @apply_index_wraps def apply_index(self, i): - shifted = tslib.shift_months(i.asi8, self.n, 'end') + shifted = tslib.shift_months(i.asi8, self.n, self._day_opt) return i._shallow_copy(shifted) def onOffset(self, dt): if self.normalize and not _is_normalized(dt): return False - days_in_month = tslib.monthrange(dt.year, dt.month)[1] - return dt.day == days_in_month + return dt.day == self._get_offset_day(dt) class MonthBegin(MonthOffset): """DateOffset of one month at beginning""" _prefix = 'MS' + _day_opt = 'start' @apply_wraps def apply(self, other): n = self.n + compare_day = self._get_offset_day(other) - if other.day > 1 and n <= 0: # then roll forward if n<=0 + if other.day > compare_day and n <= 0: # then roll forward if n<=0 n += 1 - return shift_month(other, n, 'start') + return shift_month(other, n, self._day_opt) @apply_index_wraps def apply_index(self, i): - shifted = tslib.shift_months(i.asi8, self.n, 'start') + shifted = tslib.shift_months(i.asi8, self.n, self._day_opt) return i._shallow_copy(shifted) def onOffset(self, dt): if self.normalize and not _is_normalized(dt): return False - return dt.day == 1 + return dt.day == self._get_offset_day(dt) class SemiMonthOffset(DateOffset): @@ -1177,45 +1184,43 @@ def _apply_index_days(self, i, roll): class BusinessMonthEnd(MonthOffset): """DateOffset increments between business EOM dates""" _prefix = 'BM' + _day_opt = 'business_end' @apply_wraps def apply(self, other): n = self.n - wkday, days_in_month = tslib.monthrange(other.year, other.month) - lastBDay = get_lastbday(wkday, days_in_month) + compare_day = self._get_offset_day(other) - if n > 0 and not other.day >= lastBDay: + if n > 0 and not other.day >= compare_day: n = n - 1 - elif n <= 0 and other.day > lastBDay: + elif n <= 0 and other.day > compare_day: n = n + 1 - return shift_month(other, n, 'business_end') + return shift_month(other, n, self._day_opt) class BusinessMonthBegin(MonthOffset): """DateOffset of one business month at beginning""" _prefix = 'BMS' + _day_opt = 'business_start' @apply_wraps def apply(self, other): n = self.n - wkday, _ = tslib.monthrange(other.year, other.month) - first = get_firstbday(wkday) + compare_day = self._get_offset_day(other) - if other.day > first and n <= 0: + if other.day > compare_day and n <= 0: # as if rolled forward already n += 1 - elif other.day < first and n > 0: - other = other + timedelta(days=first - other.day) + elif other.day < compare_day and n > 0: n -= 1 - return shift_month(other, n, 'business_start') + return shift_month(other, n, self._day_opt) def onOffset(self, dt): if self.normalize and not _is_normalized(dt): return False - first_weekday, _ = tslib.monthrange(dt.year, dt.month) - return dt.day == get_firstbday(first_weekday) + return dt.day == self._get_offset_day(dt) class CustomBusinessMonthEnd(BusinessMixin, MonthOffset): @@ -1363,6 +1368,9 @@ def apply(self, other): return result +# --------------------------------------------------------------------- +# Week-Based Offset Classes + class Week(EndMixin, DateOffset): """ Weekly offset @@ -1594,6 +1602,8 @@ def _from_name(cls, suffix=None): weekday = _weekday_to_int[suffix] return cls(weekday=weekday) +# --------------------------------------------------------------------- +# Quarter-Based Offset Classes class QuarterOffset(DateOffset): """Quarter representation - doesn't call super""" @@ -1641,24 +1651,23 @@ class BQuarterEnd(QuarterOffset): _default_startingMonth = 3 _from_name_startingMonth = 12 _prefix = 'BQ' + _day_opt = 'business_end' @apply_wraps def apply(self, other): n = self.n - - wkday, days_in_month = tslib.monthrange(other.year, other.month) - lastBDay = get_lastbday(wkday, days_in_month) + compare_day = self._get_offset_day(other) monthsToGo = 3 - ((other.month - self.startingMonth) % 3) if monthsToGo == 3: monthsToGo = 0 - if n > 0 and not (other.day >= lastBDay and monthsToGo == 0): + if n > 0 and not (other.day >= compare_day and monthsToGo == 0): n = n - 1 - elif n <= 0 and other.day > lastBDay and monthsToGo == 0: + elif n <= 0 and other.day > compare_day and monthsToGo == 0: n = n + 1 - return shift_month(other, monthsToGo + 3 * n, 'business_end') + return shift_month(other, monthsToGo + 3 * n, self._day_opt) def onOffset(self, dt): if self.normalize and not _is_normalized(dt): @@ -1678,13 +1687,13 @@ class BQuarterBegin(QuarterOffset): _default_startingMonth = 3 _from_name_startingMonth = 1 _prefix = 'BQS' + _day_opt = 'business_start' @apply_wraps def apply(self, other): n = self.n - wkday, _ = tslib.monthrange(other.year, other.month) - first = get_firstbday(wkday) + compare_day = self._get_offset_day(other) monthsSince = (other.month - self.startingMonth) % 3 @@ -1692,13 +1701,13 @@ def apply(self, other): monthsSince = monthsSince - 3 # roll forward if on same month later than first bday - if n <= 0 and (monthsSince == 0 and other.day > first): + if n <= 0 and (monthsSince == 0 and other.day > compare_day): n = n + 1 # pretend to roll back if on same month but before firstbday - elif n > 0 and (monthsSince == 0 and other.day < first): + elif n > 0 and (monthsSince == 0 and other.day < compare_day): n = n - 1 - return shift_month(other, 3 * n - monthsSince, 'business_start') + return shift_month(other, 3 * n - monthsSince, self._day_opt) class QuarterEnd(EndMixin, QuarterOffset): @@ -1710,6 +1719,7 @@ class QuarterEnd(EndMixin, QuarterOffset): _outputName = 'QuarterEnd' _default_startingMonth = 3 _prefix = 'Q' + _day_opt = 'end' @apply_wraps def apply(self, other): @@ -1717,16 +1727,16 @@ def apply(self, other): other = datetime(other.year, other.month, other.day, other.hour, other.minute, other.second, other.microsecond) - wkday, days_in_month = tslib.monthrange(other.year, other.month) + compare_day = self._get_offset_day(other) monthsToGo = 3 - ((other.month - self.startingMonth) % 3) if monthsToGo == 3: monthsToGo = 0 - if n > 0 and not (other.day >= days_in_month and monthsToGo == 0): + if n > 0 and not (other.day >= compare_day and monthsToGo == 0): n = n - 1 - other = shift_month(other, monthsToGo + 3 * n, 'end') + other = shift_month(other, monthsToGo + 3 * n, self._day_opt) return other @apply_index_wraps @@ -1745,11 +1755,12 @@ class QuarterBegin(BeginMixin, QuarterOffset): _default_startingMonth = 3 _from_name_startingMonth = 1 _prefix = 'QS' + _day_opt = 'start' @apply_wraps def apply(self, other): n = self.n - wkday, days_in_month = tslib.monthrange(other.year, other.month) + compare_day = self._get_offset_day(other) monthsSince = (other.month - self.startingMonth) % 3 @@ -1757,11 +1768,11 @@ def apply(self, other): # make sure you roll forward, so negate monthsSince = monthsSince - 3 - if n <= 0 and (monthsSince == 0 and other.day > 1): + if n <= 0 and (monthsSince == 0 and other.day > compare_day): # after start, so come back an extra period as if rolled forward n = n + 1 - other = shift_month(other, 3 * n - monthsSince, 'start') + other = shift_month(other, 3 * n - monthsSince, self._day_opt) return other @apply_index_wraps @@ -1771,10 +1782,19 @@ def apply_index(self, i): return self._beg_apply_index(i, freqstr) +# --------------------------------------------------------------------- +# Year-Based Offset Classes + class YearOffset(DateOffset): """DateOffset that just needs a month""" _adjust_dst = True + def _get_offset_day(self, other): + # override BaseOffset method to use self.month instead of other.month + # TODO: there may be a more performant way to do this + return liboffsets.get_day_of_month(other.replace(month=self.month), + self._day_opt) + def __init__(self, n=1, normalize=False, month=None): month = month if month is not None else self._default_month self.month = month @@ -1802,25 +1822,25 @@ class BYearEnd(YearOffset): _outputName = 'BusinessYearEnd' _default_month = 12 _prefix = 'BA' + _day_opt = 'business_end' @apply_wraps def apply(self, other): n = self.n - wkday, days_in_month = tslib.monthrange(other.year, self.month) - lastBDay = get_lastbday(wkday, days_in_month) + compare_day = self._get_offset_day(other) years = n if n > 0: if (other.month < self.month or - (other.month == self.month and other.day < lastBDay)): + (other.month == self.month and other.day < compare_day)): years -= 1 elif n <= 0: if (other.month > self.month or - (other.month == self.month and other.day > lastBDay)): + (other.month == self.month and other.day > compare_day)): years += 1 months = years * 12 + (self.month - other.month) - return shift_month(other, months, 'business_end') + return shift_month(other, months, self._day_opt) class BYearBegin(YearOffset): @@ -1828,38 +1848,38 @@ class BYearBegin(YearOffset): _outputName = 'BusinessYearBegin' _default_month = 1 _prefix = 'BAS' + _day_opt = 'business_start' @apply_wraps def apply(self, other): n = self.n - wkday, days_in_month = tslib.monthrange(other.year, self.month) - - first = get_firstbday(wkday) + compare_day = self._get_offset_day(other) years = n if n > 0: # roll back first for positive n if (other.month < self.month or - (other.month == self.month and other.day < first)): + (other.month == self.month and other.day < compare_day)): years -= 1 elif n <= 0: # roll forward if (other.month > self.month or - (other.month == self.month and other.day > first)): + (other.month == self.month and other.day > compare_day)): years += 1 # set first bday for result months = years * 12 + (self.month - other.month) - return shift_month(other, months, 'business_start') + return shift_month(other, months, self._day_opt) class YearEnd(EndMixin, YearOffset): """DateOffset increments between calendar year ends""" _default_month = 12 _prefix = 'A' + _day_opt = 'end' @apply_wraps def apply(self, other): - n = roll_yearday(other, self.n, self.month, 'end') + n = roll_yearday(other, self.n, self.month, self._day_opt) year = other.year + n days_in_month = tslib.monthrange(year, self.month)[1] return datetime(year, self.month, days_in_month, @@ -1874,18 +1894,18 @@ def apply_index(self, i): def onOffset(self, dt): if self.normalize and not _is_normalized(dt): return False - wkday, days_in_month = tslib.monthrange(dt.year, self.month) - return self.month == dt.month and dt.day == days_in_month + return self.month == dt.month and dt.day == self._get_offset_day(dt) class YearBegin(BeginMixin, YearOffset): """DateOffset increments between calendar year begin dates""" _default_month = 1 _prefix = 'AS' + _day_opt = 'start' @apply_wraps def apply(self, other): - n = roll_yearday(other, self.n, self.month, 'start') + n = roll_yearday(other, self.n, self.month, self._day_opt) year = other.year + n return other.replace(year=year, month=self.month, day=1) @@ -1898,8 +1918,11 @@ def apply_index(self, i): def onOffset(self, dt): if self.normalize and not _is_normalized(dt): return False - return dt.month == self.month and dt.day == 1 + return dt.month == self.month and dt.day == self._get_offset_day(dt) + +# --------------------------------------------------------------------- +# Special Offset Classes class FY5253(DateOffset): """ From fa57b66d9d74017ea2365baa2ff294146993e0f3 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 16 Nov 2017 10:38:42 -0800 Subject: [PATCH 2/7] whitespace fixup; remove unused imports --- pandas/tseries/offsets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index d66903ec08edb..bec16a7515df2 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -19,7 +19,6 @@ from pandas._libs.tslibs.offsets import ( ApplyTypeError, as_datetime, _is_normalized, - get_firstbday, get_lastbday, _get_calendar, _to_dt64, _validate_business_time, _int_to_weekday, _weekday_to_int, _determine_offset, @@ -1605,6 +1604,7 @@ def _from_name(cls, suffix=None): # --------------------------------------------------------------------- # Quarter-Based Offset Classes + class QuarterOffset(DateOffset): """Quarter representation - doesn't call super""" _default_startingMonth = None From 30991ba8d1c27b45ffefe2f98af20b5b0492530a Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 16 Nov 2017 10:47:20 -0800 Subject: [PATCH 3/7] comment typo fixup --- pandas/_libs/tslibs/offsets.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 507a8504b6562..d63bcd486177a 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -515,7 +515,7 @@ cpdef int get_day_of_month(datetime other, day_opt) except? -1: elif is_integer_object(day_opt): day = min(day_opt, days_in_month) else: - # Note: unlike `shift_month`, this get_day_of_month does not + # Note: unlike `shift_month`, get_day_of_month does not # allow day_opt = None raise ValueError(day_opt) From 85a3deb008281ab614d9cfb4e73b3366ea4ba791 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 16 Nov 2017 17:47:18 -0800 Subject: [PATCH 4/7] raise notimplementerror if calling _get_day_opt from base class --- pandas/_libs/tslibs/offsets.pyx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index d63bcd486177a..19697c66b3b70 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -357,6 +357,7 @@ class _BaseOffset(object): _typ = "dateoffset" _normalize_cache = True _cacheable = False + _day_opt = None def __call__(self, other): return self.apply(other) @@ -395,7 +396,8 @@ class _BaseOffset(object): return out def _get_offset_day(self, datetime other): - # subclass must implement `_day_opt` + # subclass must implement `_day_opt`; calling from the base class + # will raise NotImplementedError. return get_day_of_month(other, self._day_opt) @@ -514,6 +516,8 @@ cpdef int get_day_of_month(datetime other, day_opt) except? -1: return get_lastbday(wkday, days_in_month) elif is_integer_object(day_opt): day = min(day_opt, days_in_month) + elif day_opt is None: + raise NotImplementedError else: # Note: unlike `shift_month`, get_day_of_month does not # allow day_opt = None From 8b6f1fd4b4d713ef0b9d0ffa97ed6378a1e1d9f4 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 17 Nov 2017 06:47:49 -0800 Subject: [PATCH 5/7] test for un-overriden _get_offset_day --- pandas/tests/tseries/offsets/test_offsets.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 250e57c273603..bf00b6808047c 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -4723,3 +4723,11 @@ def test_shift_month(): # Try to shift to April 31, then shift back to Apr 30 to get a real date assert liboffsets.shift_month(ts, -1, 31) == Timestamp('1929-04-30') + + +def test_get_offset_day_error(): + # subclass of _BaseOffset must override _day_opt attribute, or we should + # get a NotImplementedError + + with pytest.raises(NotImplementedError): + DateOffset()._get_offset_day(datetime.now()) From 8906d210f6fc6f50d09643e48d6654c681a5fbef Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 17 Nov 2017 06:48:34 -0800 Subject: [PATCH 6/7] move comment --- pandas/_libs/tslibs/offsets.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 19697c66b3b70..f38aca21a0438 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -517,10 +517,10 @@ cpdef int get_day_of_month(datetime other, day_opt) except? -1: elif is_integer_object(day_opt): day = min(day_opt, days_in_month) elif day_opt is None: - raise NotImplementedError - else: # Note: unlike `shift_month`, get_day_of_month does not # allow day_opt = None + raise NotImplementedError + else: raise ValueError(day_opt) From e00414fe42c9b84ad4f1885ff362e2dd051730f6 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 17 Nov 2017 12:37:39 -0800 Subject: [PATCH 7/7] dummy commit to force CI --- pandas/tseries/offsets.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index bec16a7515df2..c9c4d1b1e7119 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -1,4 +1,7 @@ # -*- coding: utf-8 -*- +import functools +import operator + from datetime import date, datetime, timedelta from pandas.compat import range from pandas import compat @@ -28,8 +31,6 @@ BeginMixin, EndMixin, BaseOffset) -import functools -import operator __all__ = ['Day', 'BusinessDay', 'BDay', 'CustomBusinessDay', 'CDay', 'CBMonthEnd', 'CBMonthBegin',