Fix bugs in FY5253Quarter and LastWeekOfMonth

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`) 
 
 

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

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

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