From a35c10bce440919a78a443b1b381bcd1ef89af1a Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 20 Dec 2017 09:21:18 -0800 Subject: [PATCH 1/3] Fix WeekOfMonth.apply, Week.onOffset --- doc/source/whatsnew/v0.22.0.txt | 1 + pandas/tests/tseries/offsets/test_offsets.py | 34 ++++++ pandas/tseries/offsets.py | 117 ++++++++++--------- 3 files changed, 97 insertions(+), 55 deletions(-) diff --git a/doc/source/whatsnew/v0.22.0.txt b/doc/source/whatsnew/v0.22.0.txt index 0579a80aad28e..54d1e02b0226a 100644 --- a/doc/source/whatsnew/v0.22.0.txt +++ b/doc/source/whatsnew/v0.22.0.txt @@ -354,3 +354,4 @@ Other - Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`) - Bug in :class:`Timestamp` where comparison with an array of ``Timestamp`` objects would result in a ``RecursionError`` (:issue:`15183`) +- Bug in :class:`WeekOfMonth` and class:`Week` where addition and subtraction did not roll correctly (:issue:`18672`,:issue:`18510`) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 5b4c2f9d86674..b304ebff55b6e 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3147,3 +3147,37 @@ def test_require_integers(offset_types): cls = offset_types with pytest.raises(ValueError): cls(n=1.5) + + +def test_weeks_onoffset(): + # GH#18510 Week with weekday = None, normalize = False should always + # be onOffset + offset = Week(n=2, weekday=None) + ts = Timestamp('1862-01-13 09:03:34.873477378+0210', tz='Africa/Lusaka') + fast = offset.onOffset(ts) + slow = (ts + offset) - offset == ts + assert fast == slow + + # negative n + offset = Week(n=2, weekday=None) + ts = Timestamp('1856-10-24 16:18:36.556360110-0717', tz='Pacific/Easter') + fast = offset.onOffset(ts) + slow = (ts + offset) - offset == ts + assert fast == slow + + +def test_weekofmonth_onoffset(): + # GH#18864 + # Make sure that nanoseconds don't trip up onOffset (and with it apply) + offset = WeekOfMonth(n=2, week=2, weekday=0) + ts = Timestamp('1916-05-15 01:14:49.583410462+0422', tz='Asia/Qyzylorda') + fast = offset.onOffset(ts) + slow = (ts + offset) - offset == ts + assert fast == slow + + # negative n + offset = WeekOfMonth(n=-3, week=1, weekday=0) + ts = Timestamp('1980-12-08 03:38:52.878321185+0500', tz='Asia/Oral') + fast = offset.onOffset(ts) + slow = (ts + offset) - offset == ts + assert fast == slow diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 8b12b2f3ad2ce..107ce9f6ad7d5 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -112,6 +112,31 @@ def wrapper(self, other): return wrapper +def shift_day(other, days): + """ + Increment the datetime `other` by the given number of days, retaining + the time-portion of the datetime. For tz-naive datetimes this is + equivalent to adding a timedelta. For tz-aware datetimes it is similar to + dateutil's relativedelta.__add__, but handles pytz tzinfo objects. + + Parameters + ---------- + other : datetime or Timestamp + days : int + + Returns + ------- + shifted: datetime or Timestamp + """ + if other.tzinfo is None: + return other + timedelta(days=days) + + tz = other.tzinfo + naive = other.replace(tzinfo=None) + shifted = naive + timedelta(days=days) + return tslib._localize_pydatetime(shifted, tz) + + # --------------------------------------------------------------------- # DateOffset @@ -1342,6 +1367,8 @@ def apply_index(self, i): def onOffset(self, dt): if self.normalize and not _is_normalized(dt): return False + elif self.weekday is None: + return True return dt.weekday() == self.weekday @property @@ -1361,7 +1388,29 @@ def _from_name(cls, suffix=None): return cls(weekday=weekday) -class WeekOfMonth(DateOffset): +class _WeekOfMonthMixin(object): + """Mixin for methods common to WeekOfMonth and LastWeekOfMonth""" + @apply_wraps + def apply(self, other): + compare_day = self._get_offset_day(other) + + months = self.n + if months > 0 and compare_day > other.day: + months -= 1 + elif months <= 0 and compare_day < other.day: + months += 1 + + shifted = shift_month(other, months, 'start') + to_day = self._get_offset_day(shifted) + return shift_day(shifted, to_day - shifted.day) + + def onOffset(self, dt): + if self.normalize and not _is_normalized(dt): + return False + return dt.day == self._get_offset_day(dt) + + +class WeekOfMonth(_WeekOfMonthMixin, DateOffset): """ Describes monthly dates like "the Tuesday of the 2nd week of each month" @@ -1400,34 +1449,11 @@ def __init__(self, n=1, normalize=False, week=None, weekday=None): self.kwds = {'weekday': weekday, 'week': week} - @apply_wraps - def apply(self, other): - base = other - offsetOfMonth = self.getOffsetOfMonth(other) - - months = self.n - if months > 0 and offsetOfMonth > other: - months -= 1 - elif months <= 0 and offsetOfMonth < other: - months += 1 - - other = self.getOffsetOfMonth(shift_month(other, months, 'start')) - other = datetime(other.year, other.month, other.day, base.hour, - base.minute, base.second, base.microsecond) - return other - - def getOffsetOfMonth(self, dt): - w = Week(weekday=self.weekday) - d = datetime(dt.year, dt.month, 1, tzinfo=dt.tzinfo) - # TODO: Is this DST-safe? - d = w.rollforward(d) - return d + timedelta(weeks=self.week) - - def onOffset(self, dt): - if self.normalize and not _is_normalized(dt): - return False - d = datetime(dt.year, dt.month, dt.day, tzinfo=dt.tzinfo) - return d == self.getOffsetOfMonth(dt) + def _get_offset_day(self, other): + mstart = datetime(other.year, other.month, 1) + wday = mstart.weekday() + shift_days = (self.weekday - wday) % 7 + return 1 + shift_days + self.week * 7 @property def rule_code(self): @@ -1448,7 +1474,7 @@ def _from_name(cls, suffix=None): return cls(week=week, weekday=weekday) -class LastWeekOfMonth(DateOffset): +class LastWeekOfMonth(_WeekOfMonthMixin, DateOffset): """ Describes monthly dates in last week of month like "the last Tuesday of each month" @@ -1482,31 +1508,12 @@ def __init__(self, n=1, normalize=False, weekday=None): self.kwds = {'weekday': weekday} - @apply_wraps - def apply(self, other): - offsetOfMonth = self.getOffsetOfMonth(other) - - months = self.n - if months > 0 and offsetOfMonth > other: - months -= 1 - elif months <= 0 and offsetOfMonth < other: - months += 1 - - return self.getOffsetOfMonth(shift_month(other, months, 'start')) - - def getOffsetOfMonth(self, dt): - m = MonthEnd() - d = datetime(dt.year, dt.month, 1, dt.hour, dt.minute, - dt.second, dt.microsecond, tzinfo=dt.tzinfo) - eom = m.rollforward(d) - # TODO: Is this DST-safe? - w = Week(weekday=self.weekday) - return w.rollback(eom) - - def onOffset(self, dt): - if self.normalize and not _is_normalized(dt): - return False - return dt == self.getOffsetOfMonth(dt) + def _get_offset_day(self, other): + dim = ccalendar.get_days_in_month(other.year, other.month) + mend = datetime(other.year, other.month, dim) + wday = mend.weekday() + shift_days = (wday - self.weekday) % 7 + return dim - shift_days @property def rule_code(self): From 2dc8ad1e1ddd7bd1967868b15efcd3300099f74b Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 21 Dec 2017 13:56:36 -0800 Subject: [PATCH 2/3] requested whatsnew edits --- doc/source/whatsnew/v0.23.0 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/source/whatsnew/v0.23.0 b/doc/source/whatsnew/v0.23.0 index 869353d475ffc..1a3b3e751190b 100644 --- a/doc/source/whatsnew/v0.23.0 +++ b/doc/source/whatsnew/v0.23.0 @@ -282,6 +282,8 @@ Conversion - Bug in :meth:`Index.astype` with a categorical dtype where the resultant index is not converted to a :class:`CategoricalIndex` for all types of index (:issue:`18630`) - Bug in :meth:`Series.astype` and ``Categorical.astype()`` where an existing categorical data does not get updated (:issue:`10696`, :issue:`18593`) - Bug in :class:`Series` constructor with an int or float list where specifying ``dtype=str``, ``dtype='str'`` or ``dtype='U'`` failed to convert the data elements to strings (:issue:`16605`) +- Bug in :class:`Timestamp` where comparison with an array of ``Timestamp`` objects would result in a ``RecursionError`` (:issue:`15183`) +- Bug in :class:`WeekOfMonth` and class:`Week` where addition and subtraction did not roll correctly (:issue:`18510`,:issue:`18672`,:issue:`18864`) Indexing @@ -361,5 +363,3 @@ Other ^^^^^ - Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`) -- Bug in :class:`Timestamp` where comparison with an array of ``Timestamp`` objects would result in a ``RecursionError`` (:issue:`15183`) -- Bug in :class:`WeekOfMonth` and class:`Week` where addition and subtraction did not roll correctly (:issue:`18672`,:issue:`18510`) From e71374f9be4404ca72fbcb63595a0f5405b29f23 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 21 Dec 2017 14:00:01 -0800 Subject: [PATCH 3/3] docstrings --- pandas/tseries/offsets.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 107ce9f6ad7d5..54250bbf903a4 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -1450,6 +1450,18 @@ def __init__(self, n=1, normalize=False, week=None, weekday=None): self.kwds = {'weekday': weekday, 'week': week} def _get_offset_day(self, other): + """ + Find the day in the same month as other that has the same + weekday as self.weekday and is the self.week'th such day in the month. + + Parameters + ---------- + other: datetime + + Returns + ------- + day: int + """ mstart = datetime(other.year, other.month, 1) wday = mstart.weekday() shift_days = (self.weekday - wday) % 7 @@ -1509,6 +1521,18 @@ def __init__(self, n=1, normalize=False, weekday=None): self.kwds = {'weekday': weekday} def _get_offset_day(self, other): + """ + Find the day in the same month as other that has the same + weekday as self.weekday and is the last such day in the month. + + Parameters + ---------- + other: datetime + + Returns + ------- + day: int + """ dim = ccalendar.get_days_in_month(other.year, other.month) mend = datetime(other.year, other.month, dim) wday = mend.weekday()