From fe7a718752e132c16129bc82c7d85a7abdcc8ae4 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 25 Jan 2018 11:48:53 -0800 Subject: [PATCH 1/3] make kwds a property, step towards making DateOffset immutable --- pandas/_libs/tslibs/offsets.pyx | 8 ++ pandas/tests/tseries/offsets/test_offsets.py | 2 +- pandas/tseries/offsets.py | 132 +++++++++---------- 3 files changed, 73 insertions(+), 69 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index a0ac6389c0646..4fc007995efa2 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -302,6 +302,14 @@ class _BaseOffset(object): _normalize_cache = True _cacheable = False _day_opt = None + _attributes = frozenset(['n', 'normalize']) + + @property + def kwds(self): + # for backwards-compatibility + kwds = {name: getattr(self, name, None) for name in self._attributes + if name not in ['n', 'normalize']} + return {name: kwds[name] for name in kwds if kwds[name] is not None} def __call__(self, other): return self.apply(other) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index b086884ecd250..d96ebab615d12 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -218,7 +218,7 @@ def test_offset_freqstr(self, offset_types): freqstr = offset.freqstr if freqstr not in ('', - "", + "", 'LWOM-SAT', ): code = get_offset(freqstr) assert offset.rule_code == code diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index ec206e0997d0b..d4f711df16b5f 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -185,6 +185,8 @@ def __add__(date): """ _use_relativedelta = False _adjust_dst = False + _attributes = frozenset(['n', 'normalize'] + + list(liboffsets.relativedelta_kwds)) # default for prior pickles normalize = False @@ -192,9 +194,10 @@ def __add__(date): def __init__(self, n=1, normalize=False, **kwds): self.n = self._validate_n(n) self.normalize = normalize - self.kwds = kwds self._offset, self._use_relativedelta = _determine_offset(kwds) + for name in kwds: + setattr(self, name, kwds[name]) @apply_wraps def apply(self, other): @@ -238,30 +241,31 @@ def apply_index(self, i): y : DatetimeIndex """ - if not type(self) is DateOffset: + if type(self) is not DateOffset: raise NotImplementedError("DateOffset subclass {name} " "does not have a vectorized " "implementation".format( name=self.__class__.__name__)) + kwds = self.kwds relativedelta_fast = set(['years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'microseconds']) # relativedelta/_offset path only valid for base DateOffset if (self._use_relativedelta and - set(self.kwds).issubset(relativedelta_fast)): + set(kwds).issubset(relativedelta_fast)): - months = ((self.kwds.get('years', 0) * 12 + - self.kwds.get('months', 0)) * self.n) + months = ((kwds.get('years', 0) * 12 + + kwds.get('months', 0)) * self.n) if months: shifted = liboffsets.shift_months(i.asi8, months) i = i._shallow_copy(shifted) - weeks = (self.kwds.get('weeks', 0)) * self.n + weeks = (kwds.get('weeks', 0)) * self.n if weeks: i = (i.to_period('W') + weeks).to_timestamp() + \ i.to_perioddelta('W') - timedelta_kwds = {k: v for k, v in self.kwds.items() + timedelta_kwds = {k: v for k, v in kwds.items() if k in ['days', 'hours', 'minutes', 'seconds', 'microseconds']} if timedelta_kwds: @@ -273,7 +277,7 @@ def apply_index(self, i): return i + (self._offset * self.n) else: # relativedelta with other keywords - kwd = set(self.kwds) - relativedelta_fast + kwd = set(kwds) - relativedelta_fast raise NotImplementedError("DateOffset with relativedelta " "keyword(s) {kwd} not able to be " "applied vectorized".format(kwd=kwd)) @@ -284,7 +288,7 @@ def isAnchored(self): return (self.n == 1) def _params(self): - all_paras = dict(list(vars(self).items()) + list(self.kwds.items())) + all_paras = vars(self) if 'holidays' in all_paras and not all_paras['holidays']: all_paras.pop('holidays') exclude = ['kwds', 'name', 'normalize', 'calendar'] @@ -301,15 +305,8 @@ def _repr_attrs(self): exclude = set(['n', 'inc', 'normalize']) attrs = [] for attr in sorted(self.__dict__): - if attr.startswith('_'): + if attr.startswith('_') or attr == 'kwds': continue - elif attr == 'kwds': # TODO: get rid of this - kwds_new = {} - for key in self.kwds: - if not hasattr(self, key): - kwds_new[key] = self.kwds[key] - if len(kwds_new) > 0: - attrs.append('kwds={kwds_new}'.format(kwds_new=kwds_new)) elif attr not in exclude: value = getattr(self, attr) attrs.append('{attr}={value}'.format(attr=attr, value=value)) @@ -427,6 +424,30 @@ def _offset_str(self): def nanos(self): raise ValueError("{name} is a non-fixed frequency".format(name=self)) + 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'] + + if '_offset' in state and not isinstance(state['_offset'], timedelta): + # relativedelta, we need to populate using its kwds + offset = state['_offset'] + odict = offset.__dict__ + kwds = {key: odict[key] for key in odict if odict[key]} + state.update(kwds) + + self.__dict__ = state + if 'weekmask' in state and 'holidays' in state: + calendar, holidays = _get_calendar(weekmask=self.weekmask, + holidays=self.holidays, + calendar=None) + self.calendar = calendar + self.holidays = holidays + class SingleConstructorOffset(DateOffset): @classmethod @@ -450,10 +471,9 @@ def __init__(self, weekmask, holidays, calendar): # following two attributes. See DateOffset._params() # holidays, weekmask - # assumes self.kwds already exists - self.kwds['weekmask'] = self.weekmask = weekmask - self.kwds['holidays'] = self.holidays = holidays - self.kwds['calendar'] = self.calendar = calendar + self.weekmask = weekmask + self.holidays = holidays + self.calendar = calendar class BusinessMixin(object): @@ -490,23 +510,6 @@ def __getstate__(self): 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): """ @@ -514,11 +517,11 @@ class BusinessDay(BusinessMixin, SingleConstructorOffset): """ _prefix = 'B' _adjust_dst = True + _attributes = frozenset(['n', 'normalize', 'offset']) def __init__(self, n=1, normalize=False, offset=timedelta(0)): self.n = self._validate_n(n) self.normalize = normalize - self.kwds = {'offset': offset} self._offset = offset def _offset_str(self): @@ -615,10 +618,8 @@ class BusinessHourMixin(BusinessMixin): def __init__(self, start='09:00', end='17:00', offset=timedelta(0)): # must be validated here to equality check - kwds = {'offset': offset} - self.start = kwds['start'] = liboffsets._validate_business_time(start) - self.end = kwds['end'] = liboffsets._validate_business_time(end) - self.kwds.update(kwds) + self.start = liboffsets._validate_business_time(start) + self.end = liboffsets._validate_business_time(end) self._offset = offset @cache_readonly @@ -843,12 +844,12 @@ class BusinessHour(BusinessHourMixin, SingleConstructorOffset): """ _prefix = 'BH' _anchor = 0 + _attributes = frozenset(['n', 'normalize', 'start', 'end', 'offset']) def __init__(self, n=1, normalize=False, start='09:00', end='17:00', offset=timedelta(0)): self.n = self._validate_n(n) self.normalize = normalize - self.kwds = {} super(BusinessHour, self).__init__(start=start, end=end, offset=offset) @@ -872,13 +873,14 @@ class CustomBusinessDay(_CustomMixin, BusinessDay): """ _cacheable = False _prefix = 'C' + _attributes = frozenset(['n', 'normalize', + 'weekmask', 'holidays', 'calendar', 'offset']) def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', holidays=None, calendar=None, offset=timedelta(0)): self.n = self._validate_n(n) self.normalize = normalize self._offset = offset - self.kwds = {'offset': offset} _CustomMixin.__init__(self, weekmask, holidays, calendar) @@ -930,6 +932,9 @@ class CustomBusinessHour(_CustomMixin, BusinessHourMixin, """ _prefix = 'CBH' _anchor = 0 + _attributes = frozenset(['n', 'normalize', + 'weekmask', 'holidays', 'calendar', + 'start', 'end', 'offset']) def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', holidays=None, calendar=None, @@ -937,7 +942,6 @@ def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', self.n = self._validate_n(n) self.normalize = normalize self._offset = offset - self.kwds = {'offset': offset} _CustomMixin.__init__(self, weekmask, holidays, calendar) BusinessHourMixin.__init__(self, start=start, end=end, offset=offset) @@ -949,11 +953,11 @@ def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', class MonthOffset(SingleConstructorOffset): _adjust_dst = True + _attributes = frozenset(['n', 'normalize']) def __init__(self, n=1, normalize=False): self.n = self._validate_n(n) self.normalize = normalize - self.kwds = {} @property def name(self): @@ -1024,6 +1028,8 @@ class _CustomBusinessMonth(_CustomMixin, BusinessMixin, MonthOffset): calendar : pd.HolidayCalendar or np.busdaycalendar """ _cacheable = False + _attributes = frozenset(['n', 'normalize', + 'weekmask', 'holidays', 'calendar', 'offset']) onOffset = DateOffset.onOffset # override MonthOffset method apply_index = DateOffset.apply_index # override MonthOffset method @@ -1033,7 +1039,6 @@ def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', self.n = self._validate_n(n) self.normalize = normalize self._offset = offset - self.kwds = {'offset': offset} _CustomMixin.__init__(self, weekmask, holidays, calendar) @@ -1102,6 +1107,7 @@ class SemiMonthOffset(DateOffset): _adjust_dst = True _default_day_of_month = 15 _min_day_of_month = 2 + _attributes = frozenset(['n', 'normalize', 'day_of_month']) def __init__(self, n=1, normalize=False, day_of_month=None): if day_of_month is None: @@ -1115,7 +1121,6 @@ def __init__(self, n=1, normalize=False, day_of_month=None): self.n = self._validate_n(n) self.normalize = normalize - self.kwds = {'day_of_month': self.day_of_month} @classmethod def _from_name(cls, suffix=None): @@ -1319,6 +1324,7 @@ class Week(DateOffset): _adjust_dst = True _inc = timedelta(weeks=1) _prefix = 'W' + _attributes = frozenset(['n', 'normalize', 'weekday']) def __init__(self, n=1, normalize=False, weekday=None): self.n = self._validate_n(n) @@ -1330,8 +1336,6 @@ def __init__(self, n=1, normalize=False, weekday=None): raise ValueError('Day must be 0<=day<=6, got {day}' .format(day=self.weekday)) - self.kwds = {'weekday': weekday} - def isAnchored(self): return (self.n == 1 and self.weekday is not None) @@ -1450,6 +1454,7 @@ class WeekOfMonth(_WeekOfMonthMixin, DateOffset): """ _prefix = 'WOM' _adjust_dst = True + _attributes = frozenset(['n', 'normalize', 'week', 'weekday']) def __init__(self, n=1, normalize=False, week=0, weekday=0): self.n = self._validate_n(n) @@ -1467,8 +1472,6 @@ def __init__(self, n=1, normalize=False, week=0, weekday=0): raise ValueError('Week must be 0<=week<=3, got {week}' .format(week=self.week)) - 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 @@ -1526,6 +1529,7 @@ class LastWeekOfMonth(_WeekOfMonthMixin, DateOffset): """ _prefix = 'LWOM' _adjust_dst = True + _attributes = frozenset(['n', 'normalize', 'weekday']) def __init__(self, n=1, normalize=False, weekday=0): self.n = self._validate_n(n) @@ -1539,8 +1543,6 @@ def __init__(self, n=1, normalize=False, weekday=0): raise ValueError('Day must be 0<=day<=6, got {day}' .format(day=self.weekday)) - self.kwds = {'weekday': weekday} - def _get_offset_day(self, other): """ Find the day in the same month as other that has the same @@ -1584,6 +1586,7 @@ class QuarterOffset(DateOffset): _default_startingMonth = None _from_name_startingMonth = None _adjust_dst = True + _attributes = frozenset(['n', 'normalize', 'startingMonth']) # TODO: Consider combining QuarterOffset and YearOffset __init__ at some # point. Also apply_index, onOffset, rule_code if # startingMonth vs month attr names are resolved @@ -1595,8 +1598,6 @@ def __init__(self, n=1, normalize=False, startingMonth=None): startingMonth = self._default_startingMonth self.startingMonth = startingMonth - self.kwds = {'startingMonth': startingMonth} - def isAnchored(self): return (self.n == 1 and self.startingMonth is not None) @@ -1690,6 +1691,7 @@ class QuarterBegin(QuarterOffset): class YearOffset(DateOffset): """DateOffset that just needs a month""" _adjust_dst = True + _attributes = frozenset(['n', 'normalize', 'month']) def _get_offset_day(self, other): # override BaseOffset method to use self.month instead of other.month @@ -1725,8 +1727,6 @@ def __init__(self, n=1, normalize=False, month=None): if self.month < 1 or self.month > 12: raise ValueError('Month must go from 1 to 12') - self.kwds = {'month': month} - @classmethod def _from_name(cls, suffix=None): kwargs = {} @@ -1811,6 +1811,7 @@ class FY5253(DateOffset): """ _prefix = 'RE' _adjust_dst = True + _attributes = frozenset(['weekday', 'startingMonth', 'variation']) def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1, variation="nearest"): @@ -1821,9 +1822,6 @@ def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1, self.variation = variation - self.kwds = {'weekday': weekday, 'startingMonth': startingMonth, - 'variation': variation} - if self.n == 0: raise ValueError('N cannot be 0') @@ -2012,6 +2010,8 @@ class FY5253Quarter(DateOffset): _prefix = 'REQ' _adjust_dst = True + _attributes = frozenset(['weekday', 'startingMonth', 'qtr_with_extra_week', + 'variation']) def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1, qtr_with_extra_week=1, variation="nearest"): @@ -2023,10 +2023,6 @@ def __init__(self, n=1, normalize=False, weekday=0, startingMonth=1, self.qtr_with_extra_week = qtr_with_extra_week self.variation = variation - self.kwds = {'weekday': weekday, 'startingMonth': startingMonth, - 'qtr_with_extra_week': qtr_with_extra_week, - 'variation': variation} - if self.n == 0: raise ValueError('N cannot be 0') @@ -2170,11 +2166,11 @@ class Easter(DateOffset): 1583-4099. """ _adjust_dst = True + _attributes = frozenset(['n', 'normalize']) def __init__(self, n=1, normalize=False): self.n = self._validate_n(n) self.normalize = normalize - self.kwds = {} @apply_wraps def apply(self, other): @@ -2217,12 +2213,12 @@ def f(self, other): class Tick(SingleConstructorOffset): _inc = Timedelta(microseconds=1000) _prefix = 'undefined' + _attributes = frozenset(['n', 'normalize']) def __init__(self, n=1, normalize=False): # TODO: do Tick classes with normalize=True make sense? self.n = self._validate_n(n) self.normalize = normalize - self.kwds = {} __gt__ = _tick_comp(operator.gt) __ge__ = _tick_comp(operator.ge) From 533cb198e6354ebb36bbe61266dfad15eb0fcad3 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 25 Jan 2018 13:44:48 -0800 Subject: [PATCH 2/3] avoid repeated kwds call --- pandas/core/indexes/datetimes.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pandas/core/indexes/datetimes.py b/pandas/core/indexes/datetimes.py index afc86a51c02b4..453cb2208305d 100644 --- a/pandas/core/indexes/datetimes.py +++ b/pandas/core/indexes/datetimes.py @@ -71,9 +71,11 @@ def f(self): if field in ['is_month_start', 'is_month_end', 'is_quarter_start', 'is_quarter_end', 'is_year_start', 'is_year_end']: - month_kw = (self.freq.kwds.get('startingMonth', - self.freq.kwds.get('month', 12)) - if self.freq else 12) + freq = self.freq + month_kw = 12 + if freq: + kwds = freq.kwds + month_kw = kwds.get('startingMonth', kwds.get('month', 12)) result = fields.get_start_end_field(values, field, self.freqstr, month_kw) From bfae96d3576f4a6cb345d0366ee5df4927df6eee Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 26 Jan 2018 17:03:13 -0800 Subject: [PATCH 3/3] requested edits --- doc/source/whatsnew/v0.23.0.txt | 1 + pandas/tseries/offsets.py | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index 4dde76dee46a5..c934404580dee 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -312,6 +312,7 @@ Other API Changes - :func:`DatetimeIndex.shift` and :func:`TimedeltaIndex.shift` will now raise ``NullFrequencyError`` (which subclasses ``ValueError``, which was raised in older versions) when the index object frequency is ``None`` (:issue:`19147`) - Addition and subtraction of ``NaN`` from a :class:`Series` with ``dtype='timedelta64[ns]'`` will raise a ``TypeError` instead of treating the ``NaN`` as ``NaT`` (:issue:`19274`) - Set operations (union, difference...) on :class:`IntervalIndex` with incompatible index types will now raise a ``TypeError`` rather than a ``ValueError`` (:issue:`19329`) +- :class:`DateOffset` objects render more simply, e.g. "" instead of "" (:issue:`19403`) .. _whatsnew_0230.deprecations: diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index d4f711df16b5f..2e4be7fbdeebf 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -196,8 +196,7 @@ def __init__(self, n=1, normalize=False, **kwds): self.normalize = normalize self._offset, self._use_relativedelta = _determine_offset(kwds) - for name in kwds: - setattr(self, name, kwds[name]) + self.__dict__.update(kwds) @apply_wraps def apply(self, other): @@ -288,7 +287,7 @@ def isAnchored(self): return (self.n == 1) def _params(self): - all_paras = vars(self) + all_paras = self.__dict__.copy() if 'holidays' in all_paras and not all_paras['holidays']: all_paras.pop('holidays') exclude = ['kwds', 'name', 'normalize', 'calendar'] @@ -427,9 +426,9 @@ def nanos(self): def __setstate__(self, state): """Reconstruct an instance from a pickled state""" if 'offset' in state: - # Older versions have offset attribute instead of _offset + # Older (<0.22.0) versions have offset attribute instead of _offset if '_offset' in state: # pragma: no cover - raise ValueError('Unexpected key `_offset`') + raise AssertionError('Unexpected key `_offset`') state['_offset'] = state.pop('offset') state['kwds']['offset'] = state['_offset']