From edc8eb475961a223fe6a2e8f0fc0b19c0fd0590d Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 10 Nov 2017 12:17:34 -0800 Subject: [PATCH 1/6] savepoint with only one test breaking --- pandas/_libs/tslibs/offsets.pyx | 95 ++++++++++++++++++++++++++-- pandas/tests/tseries/test_offsets.py | 30 +++++---- pandas/tseries/offsets.py | 90 ++++++++------------------ 3 files changed, 136 insertions(+), 79 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 87be9fa910101..ebc57a7ce1e8c 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -10,6 +10,7 @@ from dateutil.relativedelta import relativedelta import numpy as np cimport numpy as np +from numpy cimport int64_t np.import_array() @@ -313,10 +314,59 @@ class EndMixin(object): return base + off +def __unpickle(): + return + +class PickleMixin(object): + """Handle issues with backwards compat related to making DateOffset + immutable. + """ + ''' + def __reduce__(self): + # We need to define this explicitly or else pickle tests fail + wlist_attrs = tuple(getattr(self, name) for name in sorted(self.kwds)) + tup = (self.n, self.normalize,) + wlist_attrs + return (self.__class__, tup) + # FIXME: Isn't this going to screw up on DateOffset? + + + def __reduce_ex__(self, protocol): + # python 3.6 compat + # http://bugs.python.org/issue28730 + # now __reduce_ex__ is defined and higher priority than __reduce__ + return self.__reduce__() + + ''' + + def __getstate__(self): + """Return a pickleable state""" + state = self.__dict__.copy() + + # Add attributes from the C base class that aren't in self.__dict__ + state['n'] = self.n + state['normalize'] = self.normalize + + # we don't want to actually pickle the calendar object + # as its a np.busyday; we recreate on deserilization + if 'calendar' in state: + del state['calendar'] + try: + state['kwds'].pop('calendar') + except KeyError: + pass + + return state + + +class BusinessMixin(object): + """ mixin to business types to provide related functions """ + pass + + # --------------------------------------------------------------------- # Base Classes - -class _BaseOffset(object): +@cython.auto_pickle(False) +cdef class _BaseOffset(object): """ Base class for DateOffset methods that are not overriden by subclasses and will (after pickle errors are resolved) go into a cdef class. @@ -325,6 +375,14 @@ class _BaseOffset(object): _normalize_cache = True _cacheable = False + cdef readonly: + int64_t n + bint normalize + + def __init__(self, n, normalize): + self.n = n + self.normalize = normalize + def __call__(self, other): return self.apply(other) @@ -361,8 +419,37 @@ class _BaseOffset(object): out = '<%s' % n_str + className + plural + self._repr_attrs() + '>' return out - -class BaseOffset(_BaseOffset): + def __setstate__(self, state): + """Reconstruct an instance from a pickled state""" + # Note: __setstate__ needs to be defined in the cython class otherwise + # trying to set self.n and self.normalize below will + # raise an AttributeError. + if 'normalize' not in state: + # default for prior pickles + # See GH #7748, #7789 + state['normalize'] = False + + if 'offset' in state: + # Older versions have offset attribute instead of _offset + if '_offset' in state: # pragma: no cover + raise ValueError('Unexpected key `_offset`') + state['_offset'] = state.pop('offset') + state['kwds']['offset'] = state['_offset'] + + self.n = state.pop('n') + self.normalize = state.pop('normalize') + self.__dict__ = state + + if 'weekmask' in state and 'holidays' in state: + calendar, holidays = _get_calendar(weekmask=self.weekmask, + holidays=self.holidays, + calendar=None) + self.kwds['calendar'] = self.calendar = calendar + self.kwds['holidays'] = self.holidays = holidays + self.kwds['weekmask'] = state['weekmask'] + + +class BaseOffset(_BaseOffset, PickleMixin): # Here we add __rfoo__ methods that don't play well with cdef classes def __rmul__(self, someInt): return self.__mul__(someInt) diff --git a/pandas/tests/tseries/test_offsets.py b/pandas/tests/tseries/test_offsets.py index 4fd3bba01602f..30ae113637435 100644 --- a/pandas/tests/tseries/test_offsets.py +++ b/pandas/tests/tseries/test_offsets.py @@ -539,8 +539,7 @@ def setup_method(self, method): def test_different_normalize_equals(self): # equivalent in this special case offset = BDay() - offset2 = BDay() - offset2.normalize = True + offset2 = BDay(normalize=True) assert offset == offset2 def test_repr(self): @@ -734,8 +733,7 @@ def test_constructor_errors(self): def test_different_normalize_equals(self): # equivalent in this special case offset = self._offset() - offset2 = self._offset() - offset2.normalize = True + offset2 = self._offset(normalize=True) assert offset == offset2 def test_repr(self): @@ -1426,8 +1424,7 @@ def test_constructor_errors(self): def test_different_normalize_equals(self): # equivalent in this special case offset = self._offset() - offset2 = self._offset() - offset2.normalize = True + offset2 = self._offset(normalize=True) assert offset == offset2 def test_repr(self): @@ -1667,8 +1664,7 @@ def setup_method(self, method): def test_different_normalize_equals(self): # equivalent in this special case offset = CDay() - offset2 = CDay() - offset2.normalize = True + offset2 = CDay(normalize=True) assert offset == offset2 def test_repr(self): @@ -1953,8 +1949,7 @@ class TestCustomBusinessMonthEnd(CustomBusinessMonthBase, Base): def test_different_normalize_equals(self): # equivalent in this special case offset = CBMonthEnd() - offset2 = CBMonthEnd() - offset2.normalize = True + offset2 = CBMonthEnd(normalize=True) assert offset == offset2 def test_repr(self): @@ -2067,8 +2062,7 @@ class TestCustomBusinessMonthBegin(CustomBusinessMonthBase, Base): def test_different_normalize_equals(self): # equivalent in this special case offset = CBMonthBegin() - offset2 = CBMonthBegin() - offset2.normalize = True + offset2 = CBMonthBegin(normalize=True) assert offset == offset2 def test_repr(self): @@ -4899,3 +4893,15 @@ def test_all_offset_classes(self): first = Timestamp(test_values[0], tz='US/Eastern') + offset() second = Timestamp(test_values[1], tz='US/Eastern') assert first == second + + +def test_date_offset_immutable(): + offset = offsets.MonthBegin(n=2, normalize=True) + with pytest.raises(AttributeError): + offset.n = 1 + + # Check that it didn't get changed + assert offset.n == 2 + + with pytest.raises(AttributeError): + offset.normalize = False diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index 5843aaa23be57..a1136f1f40076 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -23,7 +23,8 @@ _determine_offset, apply_index_wraps, BeginMixin, EndMixin, - BaseOffset) + BaseOffset, + BusinessMixin as _BusinessMixin) import functools import operator @@ -160,11 +161,12 @@ def __add__(date): _adjust_dst = False # default for prior pickles - normalize = False + # normalize = False + # FIXME: without commenting this out, all + # DateOffsets get initialized with normalize=False, regardless of kwarg def __init__(self, n=1, normalize=False, **kwds): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.kwds = kwds self._offset, self._use_relativedelta = _determine_offset(kwds) @@ -256,6 +258,11 @@ def isAnchored(self): def _params(self): all_paras = dict(list(vars(self).items()) + list(self.kwds.items())) + + # Add in C-class attributes not present in self.__dict__ + all_paras['n'] = self.n + all_paras['normalize'] = self.normalize + if 'holidays' in all_paras and not all_paras['holidays']: all_paras.pop('holidays') exclude = ['kwds', 'name', 'normalize', 'calendar'] @@ -408,7 +415,7 @@ def _from_name(cls, suffix=None): return cls() -class BusinessMixin(object): +class BusinessMixin(_BusinessMixin): """ mixin to business types to provide related functions """ @property @@ -427,38 +434,6 @@ def _repr_attrs(self): out += ': ' + ', '.join(attrs) return out - def __getstate__(self): - """Return a pickleable state""" - state = self.__dict__.copy() - - # we don't want to actually pickle the calendar object - # as its a np.busyday; we recreate on deserilization - if 'calendar' in state: - del state['calendar'] - try: - state['kwds'].pop('calendar') - except KeyError: - pass - - return state - - def __setstate__(self, state): - """Reconstruct an instance from a pickled state""" - if 'offset' in state: - # Older versions have offset attribute instead of _offset - if '_offset' in state: # pragma: no cover - raise ValueError('Unexpected key `_offset`') - state['_offset'] = state.pop('offset') - state['kwds']['offset'] = state['_offset'] - self.__dict__ = state - if 'weekmask' in state and 'holidays' in state: - calendar, holidays = _get_calendar(weekmask=self.weekmask, - holidays=self.holidays, - calendar=None) - self.kwds['calendar'] = self.calendar = calendar - self.kwds['holidays'] = self.holidays = holidays - self.kwds['weekmask'] = state['weekmask'] - class BusinessDay(BusinessMixin, SingleConstructorOffset): """ @@ -468,8 +443,7 @@ class BusinessDay(BusinessMixin, SingleConstructorOffset): _adjust_dst = True def __init__(self, n=1, normalize=False, offset=timedelta(0)): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.kwds = {'offset': offset} self._offset = offset @@ -776,8 +750,7 @@ class BusinessHour(BusinessHourMixin, SingleConstructorOffset): def __init__(self, n=1, normalize=False, start='09:00', end='17:00', offset=timedelta(0)): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) super(BusinessHour, self).__init__(start=start, end=end, offset=offset) @cache_readonly @@ -813,8 +786,7 @@ class CustomBusinessDay(BusinessDay): def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', holidays=None, calendar=None, offset=timedelta(0)): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self._offset = offset self.kwds = {} @@ -881,8 +853,7 @@ class CustomBusinessHour(BusinessHourMixin, SingleConstructorOffset): def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', holidays=None, calendar=None, start='09:00', end='17:00', offset=timedelta(0)): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) super(CustomBusinessHour, self).__init__(start=start, end=end, offset=offset) @@ -975,6 +946,7 @@ class SemiMonthOffset(DateOffset): _min_day_of_month = 2 def __init__(self, n=1, normalize=False, day_of_month=None): + BaseOffset.__init__(self, n, normalize) if day_of_month is None: self.day_of_month = self._default_day_of_month else: @@ -983,8 +955,8 @@ def __init__(self, n=1, normalize=False, day_of_month=None): msg = 'day_of_month must be {min}<=day_of_month<=27, got {day}' raise ValueError(msg.format(min=self._min_day_of_month, day=self.day_of_month)) - self.n = int(n) - self.normalize = normalize + # self.n = int(n) + # self.normalize = normalize self.kwds = {'day_of_month': self.day_of_month} @classmethod @@ -1259,8 +1231,7 @@ class CustomBusinessMonthEnd(BusinessMixin, MonthOffset): def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', holidays=None, calendar=None, offset=timedelta(0)): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self._offset = offset self.kwds = {} @@ -1330,8 +1301,7 @@ class CustomBusinessMonthBegin(BusinessMixin, MonthOffset): def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', holidays=None, calendar=None, offset=timedelta(0)): - self.n = int(n) - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self._offset = offset self.kwds = {} @@ -1394,8 +1364,7 @@ class Week(EndMixin, DateOffset): _prefix = 'W' def __init__(self, n=1, normalize=False, weekday=None): - self.n = n - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.weekday = weekday if self.weekday is not None: @@ -1485,8 +1454,7 @@ class WeekOfMonth(DateOffset): _adjust_dst = True def __init__(self, n=1, normalize=False, week=None, weekday=None): - self.n = n - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.weekday = weekday self.week = week @@ -1582,8 +1550,7 @@ class LastWeekOfMonth(DateOffset): _prefix = 'LWOM' def __init__(self, n=1, normalize=False, weekday=None): - self.n = n - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.weekday = weekday if self.n == 0: @@ -1656,8 +1623,7 @@ class QuarterOffset(DateOffset): # point def __init__(self, n=1, normalize=False, startingMonth=None): - self.n = n - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) if startingMonth is None: startingMonth = self._default_startingMonth self.startingMonth = startingMonth @@ -2092,8 +2058,7 @@ class FY5253(DateOffset): def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1, variation="nearest"): - self.n = n - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.startingMonth = startingMonth self.weekday = weekday @@ -2342,8 +2307,7 @@ class FY5253Quarter(DateOffset): def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1, qtr_with_extra_week=1, variation="nearest"): - self.n = n - self.normalize = normalize + BaseOffset.__init__(self, n, normalize) self.weekday = weekday self.startingMonth = startingMonth From 78a0879d445b727b755a39f574467d1a9d60c818 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 10 Nov 2017 14:54:29 -0800 Subject: [PATCH 2/6] Tests passing, saving checkpoint --- pandas/_libs/tslibs/offsets.pyx | 10 ++++++---- pandas/compat/pickle_compat.py | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index ebc57a7ce1e8c..41031ef32d598 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -379,7 +379,7 @@ cdef class _BaseOffset(object): int64_t n bint normalize - def __init__(self, n, normalize): + def __init__(self, n=1, normalize=False): self.n = n self.normalize = normalize @@ -430,17 +430,19 @@ cdef class _BaseOffset(object): state['normalize'] = False if 'offset' in state: - # Older versions have offset attribute instead of _offset + # Older versions Business offsets have offset attribute + # instead of _offset if '_offset' in state: # pragma: no cover raise ValueError('Unexpected key `_offset`') state['_offset'] = state.pop('offset') state['kwds']['offset'] = state['_offset'] - self.n = state.pop('n') - self.normalize = state.pop('normalize') + self.n = state.pop('n', 1) + self.normalize = state.pop('normalize', False) self.__dict__ = state if 'weekmask' in state and 'holidays' in state: + # Business subclasses calendar, holidays = _get_calendar(weekmask=self.weekmask, holidays=self.holidays, calendar=None) diff --git a/pandas/compat/pickle_compat.py b/pandas/compat/pickle_compat.py index 8015642919611..2c43897ef4796 100644 --- a/pandas/compat/pickle_compat.py +++ b/pandas/compat/pickle_compat.py @@ -2,6 +2,7 @@ Support pre-0.12 series pickle compatibility. """ +import inspect import sys import pandas # noqa import copy @@ -22,7 +23,6 @@ def load_reduce(self): stack[-1] = func(*args) return except Exception as e: - # If we have a deprecated function, # try to replace and try again. @@ -47,6 +47,21 @@ def load_reduce(self): except: pass + if (len(args) and inspect.isclass(args[0]) and + getattr(args[0], '_typ', None) == 'dateoffset' and + args[1] is object): + # See GH#17313 + from pandas.tseries import offsets + args = (args[0], offsets.BaseOffset,) + args[2:] + if len(args) == 3 and args[2] is None: + args = args[:2] + (1,) + # kludge + try: + stack[-1] = func(*args) + return + except: + pass + # unknown exception, re-raise if getattr(self, 'is_verbose', None): print(sys.exc_info()) From 748155c3c3c81b4d4d4dcf3dd93343ea878906c4 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 10 Nov 2017 15:29:43 -0800 Subject: [PATCH 3/6] whitespace fixup, remove commented-out --- pandas/_libs/tslibs/offsets.pyx | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 41031ef32d598..2c5b81ba33b37 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -317,26 +317,11 @@ class EndMixin(object): def __unpickle(): return + class PickleMixin(object): """Handle issues with backwards compat related to making DateOffset immutable. """ - ''' - def __reduce__(self): - # We need to define this explicitly or else pickle tests fail - wlist_attrs = tuple(getattr(self, name) for name in sorted(self.kwds)) - tup = (self.n, self.normalize,) + wlist_attrs - return (self.__class__, tup) - # FIXME: Isn't this going to screw up on DateOffset? - - - def __reduce_ex__(self, protocol): - # python 3.6 compat - # http://bugs.python.org/issue28730 - # now __reduce_ex__ is defined and higher priority than __reduce__ - return self.__reduce__() - - ''' def __getstate__(self): """Return a pickleable state""" From 50e5ab25eb126e9ef6bba244f69ecb31566f0ac2 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 10 Nov 2017 15:37:27 -0800 Subject: [PATCH 4/6] remove unused --- pandas/_libs/tslibs/offsets.pyx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 2c5b81ba33b37..6e72cf159df63 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -314,10 +314,6 @@ class EndMixin(object): return base + off -def __unpickle(): - return - - class PickleMixin(object): """Handle issues with backwards compat related to making DateOffset immutable. From 95784a11148aa08e90b7c117d24857711fd38d48 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 10 Nov 2017 15:39:29 -0800 Subject: [PATCH 5/6] merge PickleMixin back into BaseOffset --- pandas/_libs/tslibs/offsets.pyx | 46 ++++++++++++++------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 6e72cf159df63..cbe758549fccd 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -314,31 +314,6 @@ class EndMixin(object): return base + off -class PickleMixin(object): - """Handle issues with backwards compat related to making DateOffset - immutable. - """ - - def __getstate__(self): - """Return a pickleable state""" - state = self.__dict__.copy() - - # Add attributes from the C base class that aren't in self.__dict__ - state['n'] = self.n - state['normalize'] = self.normalize - - # we don't want to actually pickle the calendar object - # as its a np.busyday; we recreate on deserilization - if 'calendar' in state: - del state['calendar'] - try: - state['kwds'].pop('calendar') - except KeyError: - pass - - return state - - class BusinessMixin(object): """ mixin to business types to provide related functions """ pass @@ -431,8 +406,27 @@ cdef class _BaseOffset(object): self.kwds['holidays'] = self.holidays = holidays self.kwds['weekmask'] = state['weekmask'] + def __getstate__(self): + """Return a pickleable state""" + state = self.__dict__.copy() + + # Add attributes from the C base class that aren't in self.__dict__ + state['n'] = self.n + state['normalize'] = self.normalize + + # we don't want to actually pickle the calendar object + # as its a np.busyday; we recreate on deserilization + if 'calendar' in state: + del state['calendar'] + try: + state['kwds'].pop('calendar') + except KeyError: + pass + + return state + -class BaseOffset(_BaseOffset, PickleMixin): +class BaseOffset(_BaseOffset): # Here we add __rfoo__ methods that don't play well with cdef classes def __rmul__(self, someInt): return self.__mul__(someInt) From bc8bfd5308d8f904b446cebb99ce882ef29c3132 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 10 Nov 2017 15:52:50 -0800 Subject: [PATCH 6/6] Make sure _use_relativedelta is set for unpickled --- pandas/_libs/tslibs/offsets.pyx | 7 ++----- pandas/compat/pickle_compat.py | 1 + pandas/tseries/offsets.py | 11 ++--------- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index cbe758549fccd..c7f43d6c10a7a 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -314,11 +314,6 @@ class EndMixin(object): return base + off -class BusinessMixin(object): - """ mixin to business types to provide related functions """ - pass - - # --------------------------------------------------------------------- # Base Classes @cython.auto_pickle(False) @@ -384,6 +379,8 @@ cdef class _BaseOffset(object): # default for prior pickles # See GH #7748, #7789 state['normalize'] = False + if '_use_relativedelta' not in state: + state['_use_relativedelta'] = False if 'offset' in state: # Older versions Business offsets have offset attribute diff --git a/pandas/compat/pickle_compat.py b/pandas/compat/pickle_compat.py index 2c43897ef4796..dbe137de1ce2b 100644 --- a/pandas/compat/pickle_compat.py +++ b/pandas/compat/pickle_compat.py @@ -62,6 +62,7 @@ def load_reduce(self): except: pass + # unknown exception, re-raise if getattr(self, 'is_verbose', None): print(sys.exc_info()) diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index a1136f1f40076..80dbd95f51621 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -23,8 +23,7 @@ _determine_offset, apply_index_wraps, BeginMixin, EndMixin, - BaseOffset, - BusinessMixin as _BusinessMixin) + BaseOffset) import functools import operator @@ -157,14 +156,8 @@ def __add__(date): Since 0 is a bit weird, we suggest avoiding its use. """ - _use_relativedelta = False _adjust_dst = False - # default for prior pickles - # normalize = False - # FIXME: without commenting this out, all - # DateOffsets get initialized with normalize=False, regardless of kwarg - def __init__(self, n=1, normalize=False, **kwds): BaseOffset.__init__(self, n, normalize) self.kwds = kwds @@ -415,7 +408,7 @@ def _from_name(cls, suffix=None): return cls() -class BusinessMixin(_BusinessMixin): +class BusinessMixin(object): """ mixin to business types to provide related functions """ @property