diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index ea4245cb3281e..924c91ef09c97 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -307,6 +307,7 @@ Conversion - Bug in :class:`FY5253` where ``datetime`` addition and subtraction incremented incorrectly for dates on the year-end but not normalized to midnight (:issue:`18854`) - Bug in :class:`DatetimeIndex` where adding or subtracting an array-like of ``DateOffset`` objects either raised (``np.array``, ``pd.Index``) or broadcast incorrectly (``pd.Series``) (:issue:`18849`) - Bug in :class:`Series` floor-division where operating on a scalar ``timedelta`` raises an exception (:issue:`18846`) +- Bug in :class:`FY5253Quarter`, :class:`LastWeekOfMonth` where rollback and rollforward behavior was inconsistent with addition and subtraction behavior (:issue:`18854`) - Bug in :class:`Index` constructor with ``dtype=CategoricalDtype(...)`` where ``categories`` and ``ordered`` are not maintained (issue:`19032`) diff --git a/pandas/tests/tseries/offsets/test_fiscal.py b/pandas/tests/tseries/offsets/test_fiscal.py index 09206439e9996..f71480e1f83a5 100644 --- a/pandas/tests/tseries/offsets/test_fiscal.py +++ b/pandas/tests/tseries/offsets/test_fiscal.py @@ -633,3 +633,25 @@ def test_fy5253_nearest_onoffset(): fast = offset.onOffset(ts) slow = (ts + offset) - offset == ts assert fast == slow + + +def test_fy5253qtr_onoffset_nearest(): + # GH#19036 + ts = Timestamp('1985-09-02 23:57:46.232550356-0300', + tz='Atlantic/Bermuda') + offset = FY5253Quarter(n=3, qtr_with_extra_week=1, startingMonth=2, + variation="nearest", weekday=0) + fast = offset.onOffset(ts) + slow = (ts + offset) - offset == ts + assert fast == slow + + +def test_fy5253qtr_onoffset_last(): + # GH#19036 + offset = FY5253Quarter(n=-2, qtr_with_extra_week=1, + startingMonth=7, variation="last", weekday=2) + ts = Timestamp('2011-01-26 19:03:40.331096129+0200', + tz='Africa/Windhoek') + slow = (ts + offset) - offset == ts + fast = offset.onOffset(ts) + assert fast == slow diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 7c7e5c4a5a35c..1a032182319f2 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3153,3 +3153,21 @@ def test_weekofmonth_onoffset(): fast = offset.onOffset(ts) slow = (ts + offset) - offset == ts assert fast == slow + + +def test_last_week_of_month_on_offset(): + # GH#19036, GH#18977 _adjust_dst was incorrect for LastWeekOfMonth + offset = LastWeekOfMonth(n=4, weekday=6) + ts = Timestamp('1917-05-27 20:55:27.084284178+0200', + tz='Europe/Warsaw') + slow = (ts + offset) - offset == ts + fast = offset.onOffset(ts) + assert fast == slow + + # negative n + offset = LastWeekOfMonth(n=-4, weekday=5) + ts = Timestamp('2005-08-27 05:01:42.799392561-0500', + tz='America/Rainy_River') + slow = (ts + offset) - offset == ts + fast = offset.onOffset(ts) + assert fast == slow diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 4f3c24ba534ff..7c5fe2f0314e4 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -1468,6 +1468,7 @@ class LastWeekOfMonth(_WeekOfMonthMixin, DateOffset): """ _prefix = 'LWOM' + _adjust_dst = True def __init__(self, n=1, normalize=False, weekday=None): self.n = self._validate_n(n) @@ -1727,8 +1728,7 @@ class FY5253(DateOffset): such as retail, manufacturing and parking industry. For more information see: - http://en.wikipedia.org/wiki/4%E2%80%934%E2%80%935_calendar - + http://en.wikipedia.org/wiki/4-4-5_calendar The year may either: - end on the last X day of the Y month. @@ -1922,7 +1922,7 @@ class FY5253Quarter(DateOffset): such as retail, manufacturing and parking industry. For more information see: - http://en.wikipedia.org/wiki/4%E2%80%934%E2%80%935_calendar + http://en.wikipedia.org/wiki/4-4-5_calendar The year may either: - end on the last X day of the Y month. @@ -1982,46 +1982,77 @@ def _offset(self): def isAnchored(self): return self.n == 1 and self._offset.isAnchored() + def _rollback_to_year(self, other): + """roll `other` back to the most recent date that was on a fiscal year + end. Return the date of that year-end, the number of full quarters + elapsed between that year-end and other, and the remaining Timedelta + since the most recent quarter-end. + + Parameters + ---------- + other : datetime or Timestamp + + Returns + ------- + tuple of + prev_year_end : Timestamp giving most recent fiscal year end + num_qtrs : int + tdelta : Timedelta + """ + num_qtrs = 0 + + norm = Timestamp(other).tz_localize(None) + start = self._offset.rollback(norm) + # Note: start <= norm and self._offset.onOffset(start) + + if start < norm: + # roll adjustment + qtr_lens = self.get_weeks(norm) + + # check thet qtr_lens is consistent with self._offset addition + end = shift_day(start, days=7 * sum(qtr_lens)) + assert self._offset.onOffset(end), (start, end, qtr_lens) + + tdelta = norm - start + for qlen in qtr_lens: + if qlen * 7 <= tdelta.days: + num_qtrs += 1 + tdelta -= Timedelta(days=qlen * 7) + else: + break + else: + tdelta = Timedelta(0) + + # Note: we always have tdelta.value >= 0 + return start, num_qtrs, tdelta + @apply_wraps def apply(self, other): - base = other + # Note: self.n == 0 is not allowed. n = self.n - if n > 0: - while n > 0: - if not self._offset.onOffset(other): - qtr_lens = self.get_weeks(other) - start = other - self._offset - else: - start = other - qtr_lens = self.get_weeks(other + self._offset) + prev_year_end, num_qtrs, tdelta = self._rollback_to_year(other) + res = prev_year_end + n += num_qtrs + if self.n <= 0 and tdelta.value > 0: + n += 1 - for weeks in qtr_lens: - start += timedelta(weeks=weeks) - if start > other: - other = start - n -= 1 - break + # Possible speedup by handling years first. + years = n // 4 + if years: + res += self._offset * years + n -= years * 4 - else: - n = -n - while n > 0: - if not self._offset.onOffset(other): - qtr_lens = self.get_weeks(other) - end = other + self._offset - else: - end = other - qtr_lens = self.get_weeks(other) - - for weeks in reversed(qtr_lens): - end -= timedelta(weeks=weeks) - if end < other: - other = end - n -= 1 - break - other = datetime(other.year, other.month, other.day, - base.hour, base.minute, base.second, base.microsecond) - return other + # Add an extra day to make *sure* we are getting the quarter lengths + # for the upcoming year, not the previous year + qtr_lens = self.get_weeks(res + Timedelta(days=1)) + + # Note: we always have 0 <= n < 4 + weeks = sum(qtr_lens[:n]) + if weeks: + res = shift_day(res, days=weeks * 7) + + return res def get_weeks(self, dt): ret = [13] * 4 @@ -2034,16 +2065,15 @@ def get_weeks(self, dt): return ret def year_has_extra_week(self, dt): - if self._offset.onOffset(dt): - prev_year_end = dt - self._offset - next_year_end = dt - else: - next_year_end = dt + self._offset - prev_year_end = dt - self._offset - - week_in_year = (next_year_end - prev_year_end).days / 7 + # Avoid round-down errors --> normalize to get + # e.g. '370D' instead of '360D23H' + norm = Timestamp(dt).normalize().tz_localize(None) - return week_in_year == 53 + next_year_end = self._offset.rollforward(norm) + prev_year_end = norm - self._offset + weeks_in_year = (next_year_end - prev_year_end).days / 7 + assert weeks_in_year in [52, 53], weeks_in_year + return weeks_in_year == 53 def onOffset(self, dt): if self.normalize and not _is_normalized(dt): @@ -2056,8 +2086,8 @@ def onOffset(self, dt): qtr_lens = self.get_weeks(dt) current = next_year_end - for qtr_len in qtr_lens[0:4]: - current += timedelta(weeks=qtr_len) + for qtr_len in qtr_lens: + current = shift_day(current, days=qtr_len * 7) if dt == current: return True return False