From 93c96d4e18f962d1b65d9abc490df6e69bdcbd8a Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Tue, 2 Jan 2018 10:04:22 -0800 Subject: [PATCH 1/5] fix bugs in LastWeekofMonth, FY5253Quarter --- pandas/tests/tseries/offsets/test_fiscal.py | 20 +++ pandas/tests/tseries/offsets/test_offsets.py | 18 +++ pandas/tseries/offsets.py | 159 +++++++++++-------- 3 files changed, 127 insertions(+), 70 deletions(-) diff --git a/pandas/tests/tseries/offsets/test_fiscal.py b/pandas/tests/tseries/offsets/test_fiscal.py index 09206439e9996..ffb20d64da9be 100644 --- a/pandas/tests/tseries/offsets/test_fiscal.py +++ b/pandas/tests/tseries/offsets/test_fiscal.py @@ -633,3 +633,23 @@ def test_fy5253_nearest_onoffset(): fast = offset.onOffset(ts) slow = (ts + offset) - offset == ts assert fast == slow + + +def test_fy5253qtr_onoffset_nearest(): + 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(): + 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 b304ebff55b6e..fac9e4926fedb 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3181,3 +3181,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#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 0e6a2259274ed..ff0a1ef5ad9e8 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -1505,6 +1505,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) @@ -1755,10 +1756,7 @@ class YearBegin(YearOffset): # --------------------------------------------------------------------- # Special Offset Classes -class FY5253(DateOffset): - """ - Describes 52-53 week fiscal year. This is also known as a 4-4-5 calendar. - +_5253doc = """ It is used by companies that desire that their fiscal year always end on the same day of the week. @@ -1767,8 +1765,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. @@ -1776,6 +1773,14 @@ class FY5253(DateOffset): X is a specific day of the week. Y is a certain month of the year +""" + + +class FY5253(DateOffset): + __doc__ = """ + Describes 52-53 week fiscal year. This is also known as a 4-4-5 calendar. + + {shared} Parameters ---------- @@ -1791,7 +1796,7 @@ class FY5253(DateOffset): startingMonth : The month in which fiscal years end. {1, 2, ... 12} variation : str {"nearest", "last"} for "LastOfMonth" or "NearestEndMonth" - """ + """.format(shared=_5253doc) _prefix = 'RE' _adjust_dst = True @@ -1950,26 +1955,11 @@ def _from_name(cls, *args): class FY5253Quarter(DateOffset): - """ + __doc__ = """ DateOffset increments between business quarter dates for 52-53 week fiscal year (also known as a 4-4-5 calendar). - It is used by companies that desire that their - fiscal year always end on the same day of the week. - - It is a method of managing accounting periods. - It is a common calendar structure for some industries, - such as retail, manufacturing and parking industry. - - For more information see: - http://en.wikipedia.org/wiki/4%E2%80%934%E2%80%935_calendar - - The year may either: - - end on the last X day of the Y month. - - end on the last X day closest to the last day of the Y month. - - X is a specific day of the week. - Y is a certain month of the year + {shared} startingMonth = 1 corresponds to dates like 1/31/2007, 4/30/2007, ... startingMonth = 2 corresponds to dates like 2/28/2007, 5/31/2007, ... @@ -1991,7 +1981,7 @@ class FY5253Quarter(DateOffset): or 14 week when needed. {1,2,3,4} variation : str {"nearest", "last"} for "LastOfMonth" or "NearestEndMonth" - """ + """.format(shared=_5253doc) _prefix = 'REQ' _adjust_dst = True @@ -2022,46 +2012,76 @@ 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 + ------- + 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 @@ -2074,16 +2094,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): @@ -2096,8 +2115,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 From f96284561438999e7f50cc2234e24e7bf0aabb70 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Tue, 2 Jan 2018 10:07:41 -0800 Subject: [PATCH 2/5] Whatsnew note --- doc/source/whatsnew/v0.23.0.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index bd3bee507baa3..1c351b0825a0c 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -300,6 +300,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`) Indexing From 4fb19bafb42cbb5e07c7e26f31396066dae92958 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Tue, 2 Jan 2018 12:36:59 -0800 Subject: [PATCH 3/5] change docstring format syntax --- pandas/tseries/offsets.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index cabd98035ae0c..b229e85bbb778 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -1740,7 +1740,7 @@ class FY5253(DateOffset): __doc__ = """ Describes 52-53 week fiscal year. This is also known as a 4-4-5 calendar. - {shared} + %s Parameters ---------- @@ -1756,7 +1756,7 @@ class FY5253(DateOffset): startingMonth : The month in which fiscal years end. {1, 2, ... 12} variation : str {"nearest", "last"} for "LastOfMonth" or "NearestEndMonth" - """.format(shared=_5253doc) + """ % _5253doc _prefix = 'RE' _adjust_dst = True @@ -1919,7 +1919,7 @@ class FY5253Quarter(DateOffset): DateOffset increments between business quarter dates for 52-53 week fiscal year (also known as a 4-4-5 calendar). - {shared} + %s startingMonth = 1 corresponds to dates like 1/31/2007, 4/30/2007, ... startingMonth = 2 corresponds to dates like 2/28/2007, 5/31/2007, ... @@ -1941,7 +1941,7 @@ class FY5253Quarter(DateOffset): or 14 week when needed. {1,2,3,4} variation : str {"nearest", "last"} for "LastOfMonth" or "NearestEndMonth" - """.format(shared=_5253doc) + """ % _5253doc _prefix = 'REQ' _adjust_dst = True From cfe3341e7cb8403ab8a8033b1e638561aa4d5aee Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Tue, 2 Jan 2018 14:24:59 -0800 Subject: [PATCH 4/5] add GH reference --- pandas/tests/tseries/offsets/test_fiscal.py | 2 ++ pandas/tests/tseries/offsets/test_offsets.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pandas/tests/tseries/offsets/test_fiscal.py b/pandas/tests/tseries/offsets/test_fiscal.py index ffb20d64da9be..f71480e1f83a5 100644 --- a/pandas/tests/tseries/offsets/test_fiscal.py +++ b/pandas/tests/tseries/offsets/test_fiscal.py @@ -636,6 +636,7 @@ def test_fy5253_nearest_onoffset(): 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, @@ -646,6 +647,7 @@ def test_fy5253qtr_onoffset_nearest(): 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', diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 5368640c6d4d0..1a032182319f2 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3156,7 +3156,7 @@ def test_weekofmonth_onoffset(): def test_last_week_of_month_on_offset(): - # GH#18977 _adjust_dst was incorrect for LastWeekOfMonth + # 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') From 8b0c604e71c7c4fc2846e18f5d805f4bdbe9d689 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 3 Jan 2018 17:02:03 -0800 Subject: [PATCH 5/5] requested edit --- pandas/tseries/offsets.py | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index b229e85bbb778..7c5fe2f0314e4 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -1716,7 +1716,10 @@ class YearBegin(YearOffset): # --------------------------------------------------------------------- # Special Offset Classes -_5253doc = """ +class FY5253(DateOffset): + """ + Describes 52-53 week fiscal year. This is also known as a 4-4-5 calendar. + It is used by companies that desire that their fiscal year always end on the same day of the week. @@ -1733,14 +1736,6 @@ class YearBegin(YearOffset): X is a specific day of the week. Y is a certain month of the year -""" - - -class FY5253(DateOffset): - __doc__ = """ - Describes 52-53 week fiscal year. This is also known as a 4-4-5 calendar. - - %s Parameters ---------- @@ -1756,7 +1751,7 @@ class FY5253(DateOffset): startingMonth : The month in which fiscal years end. {1, 2, ... 12} variation : str {"nearest", "last"} for "LastOfMonth" or "NearestEndMonth" - """ % _5253doc + """ _prefix = 'RE' _adjust_dst = True @@ -1915,11 +1910,26 @@ def _from_name(cls, *args): class FY5253Quarter(DateOffset): - __doc__ = """ + """ DateOffset increments between business quarter dates for 52-53 week fiscal year (also known as a 4-4-5 calendar). - %s + It is used by companies that desire that their + fiscal year always end on the same day of the week. + + It is a method of managing accounting periods. + It is a common calendar structure for some industries, + such as retail, manufacturing and parking industry. + + For more information see: + http://en.wikipedia.org/wiki/4-4-5_calendar + + The year may either: + - end on the last X day of the Y month. + - end on the last X day closest to the last day of the Y month. + + X is a specific day of the week. + Y is a certain month of the year startingMonth = 1 corresponds to dates like 1/31/2007, 4/30/2007, ... startingMonth = 2 corresponds to dates like 2/28/2007, 5/31/2007, ... @@ -1941,7 +1951,7 @@ class FY5253Quarter(DateOffset): or 14 week when needed. {1,2,3,4} variation : str {"nearest", "last"} for "LastOfMonth" or "NearestEndMonth" - """ % _5253doc + """ _prefix = 'REQ' _adjust_dst = True @@ -1984,6 +1994,7 @@ def _rollback_to_year(self, other): Returns ------- + tuple of prev_year_end : Timestamp giving most recent fiscal year end num_qtrs : int tdelta : Timedelta