-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Bitesize offsets #17318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bitesize offsets #17318
Changes from 12 commits
3822f99
309dd54
87f659b
3d1e6f8
0f5b2a6
c66b842
f016855
1fbe61d
030afde
2ca3bf2
1f4b5cd
d1e9161
3d3c2a6
24f6eee
c22ee24
e52a791
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# -*- coding: utf-8 -*- | ||
from datetime import date, datetime, timedelta | ||
from pandas.compat import range | ||
from pandas import compat | ||
|
@@ -323,37 +324,42 @@ def _params(self): | |
|
||
def __repr__(self): | ||
className = getattr(self, '_outputName', type(self).__name__) | ||
|
||
if abs(self.n) != 1: | ||
plural = 's' | ||
else: | ||
plural = '' | ||
|
||
n_str = "" | ||
if self.n != 1: | ||
n_str = "%s * " % self.n | ||
|
||
out = '<%s' % n_str + className + plural + self._repr_attrs() + '>' | ||
return out | ||
|
||
# TODO: Combine this with BusinessMixin version by defining a whitelisted | ||
# set of attributes on each object rather than the existing behavior of | ||
# iterating over internal ``__dict__`` | ||
def _repr_attrs(self): | ||
exclude = set(['n', 'inc', 'normalize']) | ||
attrs = [] | ||
for attr in sorted(self.__dict__): | ||
if ((attr == 'kwds' and len(self.kwds) == 0) or | ||
attr.startswith('_')): | ||
if attr.startswith('_'): | ||
continue | ||
elif attr == 'kwds': | ||
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('='.join((attr, repr(kwds_new)))) | ||
else: | ||
if attr not in exclude: | ||
attrs.append('='.join((attr, repr(getattr(self, attr))))) | ||
|
||
plural = '' | ||
if abs(self.n) != 1: | ||
plural = 's' | ||
|
||
n_str = '' | ||
if self.n != 1: | ||
n_str = '{n} * '.format(n=self.n) | ||
attrs.append('kwds=%s' % (kwds_new)) | ||
elif attr not in exclude: | ||
value = getattr(self, attr) | ||
attrs.append('%s=%s' % (attr, value)) | ||
|
||
attrs_str = '' | ||
out = '' | ||
if attrs: | ||
attrs_str = ': ' + ', '.join(attrs) | ||
|
||
repr_content = ''.join([n_str, className, plural, attrs_str]) | ||
out = '<{content}>'.format(content=repr_content) | ||
out += ': ' + ', '.join(attrs) | ||
return out | ||
|
||
@property | ||
|
@@ -507,8 +513,18 @@ def freqstr(self): | |
else: | ||
fstr = code | ||
|
||
try: | ||
if self._offset: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this still needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Not all subclasses define the |
||
fstr += self._offset_str() | ||
except AttributeError: | ||
# TODO: standardize `_offset` vs `offset` naming convention | ||
pass | ||
|
||
return fstr | ||
|
||
def _offset_str(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try not to add new things which just clutter up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return '' | ||
|
||
@property | ||
def nanos(self): | ||
raise ValueError("{name} is a non-fixed frequency".format(name=self)) | ||
|
@@ -527,23 +543,10 @@ def _from_name(cls, suffix=None): | |
class BusinessMixin(object): | ||
""" mixin to business types to provide related functions """ | ||
|
||
# TODO: Combine this with DateOffset by defining a whitelisted set of | ||
# attributes on each object rather than the existing behavior of iterating | ||
# over internal ``__dict__`` | ||
def __repr__(self): | ||
className = getattr(self, '_outputName', self.__class__.__name__) | ||
|
||
plural = '' | ||
if abs(self.n) != 1: | ||
plural = 's' | ||
|
||
n_str = '' | ||
if self.n != 1: | ||
n_str = '{n} * '.format(n=self.n) | ||
|
||
repr_content = ''.join([n_str, className, plural, self._repr_attrs()]) | ||
out = '<{content}>'.format(content=repr_content) | ||
return out | ||
@property | ||
def offset(self): | ||
# Alias for backward compat | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better comment on what this attribute is (its not for backward compat, rather its the API). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand. It explicitly is for backward compat since we are trying to standardize on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. Though to the extent that we can lock down what constitutes the user-facing API, |
||
return self._offset | ||
|
||
def _repr_attrs(self): | ||
if self.offset: | ||
|
@@ -572,6 +575,11 @@ def __getstate__(self): | |
|
||
def __setstate__(self, state): | ||
"""Reconstruct an instance from a pickled state""" | ||
if 'offset' in state: | ||
# Older versions have offset attribute instead of _offset | ||
assert '_offset' not in state, list(state.keys()) | ||
state['_offset'] = state['offset'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
can you remove the assert? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you may want to add a 0.20.3 pickle that adds things for every frequency. in separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll make a note to do this once other fixes to offsets are done. |
||
del state['offset'] | ||
self.__dict__ = state | ||
if 'weekmask' in state and 'holidays' in state: | ||
calendar, holidays = _get_calendar(weekmask=self.weekmask, | ||
|
@@ -593,24 +601,7 @@ def __init__(self, n=1, normalize=False, **kwds): | |
self.n = int(n) | ||
self.normalize = normalize | ||
self.kwds = kwds | ||
self.offset = kwds.get('offset', timedelta(0)) | ||
|
||
@property | ||
def freqstr(self): | ||
try: | ||
code = self.rule_code | ||
except NotImplementedError: | ||
return repr(self) | ||
|
||
if self.n != 1: | ||
fstr = '{n}{code}'.format(n=self.n, code=code) | ||
else: | ||
fstr = code | ||
|
||
if self.offset: | ||
fstr += self._offset_str() | ||
|
||
return fstr | ||
self._offset = kwds.get('offset', timedelta(0)) | ||
|
||
def _offset_str(self): | ||
def get_str(td): | ||
|
@@ -643,9 +634,6 @@ def get_str(td): | |
else: | ||
return '+' + repr(self.offset) | ||
|
||
def isAnchored(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pls don't simply remove things. instead in a separate PR deprecate them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method is still available. It's inherited verbatim from the parent class. |
||
return (self.n == 1) | ||
|
||
@apply_wraps | ||
def apply(self, other): | ||
if isinstance(other, datetime): | ||
|
@@ -709,7 +697,7 @@ def __init__(self, **kwds): | |
kwds['start'] = self._validate_time(kwds.get('start', '09:00')) | ||
kwds['end'] = self._validate_time(kwds.get('end', '17:00')) | ||
self.kwds = kwds | ||
self.offset = kwds.get('offset', timedelta(0)) | ||
self._offset = kwds.get('offset', timedelta(0)) | ||
self.start = kwds.get('start', '09:00') | ||
self.end = kwds.get('end', '17:00') | ||
|
||
|
@@ -776,7 +764,7 @@ def _get_business_hours_by_sec(self): | |
Return business hours in a day by seconds. | ||
""" | ||
if self._get_daytime_flag(): | ||
# create dummy datetime to calcurate businesshours in a day | ||
# create dummy datetime to calculate businesshours in a day | ||
dtstart = datetime(2014, 4, 1, self.start.hour, self.start.minute) | ||
until = datetime(2014, 4, 1, self.end.hour, self.end.minute) | ||
return (until - dtstart).total_seconds() | ||
|
@@ -811,7 +799,7 @@ def rollforward(self, dt): | |
|
||
@apply_wraps | ||
def apply(self, other): | ||
# calcurate here because offset is not immutable | ||
# calculate here because offset is not immutable | ||
daytime = self._get_daytime_flag() | ||
businesshours = self._get_business_hours_by_sec() | ||
bhdelta = timedelta(seconds=businesshours) | ||
|
@@ -860,7 +848,7 @@ def apply(self, other): | |
if n >= 0: | ||
bday_edge = self._prev_opening_time(other) | ||
bday_edge = bday_edge + bhdelta | ||
# calcurate remainder | ||
# calculate remainder | ||
bday_remain = result - bday_edge | ||
result = self._next_opening_time(other) | ||
result += bday_remain | ||
|
@@ -898,7 +886,7 @@ def onOffset(self, dt): | |
|
||
def _onOffset(self, dt, businesshours): | ||
""" | ||
Slight speedups using calcurated values | ||
Slight speedups using calculated values | ||
""" | ||
# if self.normalize and not _is_normalized(dt): | ||
# return False | ||
|
@@ -980,7 +968,8 @@ def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', | |
self.n = int(n) | ||
self.normalize = normalize | ||
self.kwds = kwds | ||
self.offset = kwds.get('offset', timedelta(0)) | ||
self._offset = kwds.get('offset', timedelta(0)) | ||
|
||
calendar, holidays = _get_calendar(weekmask=weekmask, | ||
holidays=holidays, | ||
calendar=calendar) | ||
|
@@ -1342,9 +1331,6 @@ def _apply_index_days(self, i, roll): | |
class BusinessMonthEnd(MonthOffset): | ||
"""DateOffset increments between business EOM dates""" | ||
|
||
def isAnchored(self): | ||
return (self.n == 1) | ||
|
||
@apply_wraps | ||
def apply(self, other): | ||
n = self.n | ||
|
@@ -1434,7 +1420,7 @@ def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', | |
self.n = int(n) | ||
self.normalize = normalize | ||
self.kwds = kwds | ||
self.offset = kwds.get('offset', timedelta(0)) | ||
self._offset = kwds.get('offset', timedelta(0)) | ||
|
||
calendar, holidays = _get_calendar(weekmask=weekmask, | ||
holidays=holidays, | ||
|
@@ -1508,7 +1494,7 @@ def __init__(self, n=1, normalize=False, weekmask='Mon Tue Wed Thu Fri', | |
self.n = int(n) | ||
self.normalize = normalize | ||
self.kwds = kwds | ||
self.offset = kwds.get('offset', timedelta(0)) | ||
self._offset = kwds.get('offset', timedelta(0)) | ||
|
||
# _get_calendar does validation and possible transformation | ||
# of calendar and holidays. | ||
|
@@ -1979,9 +1965,6 @@ class QuarterEnd(QuarterOffset): | |
_default_startingMonth = 3 | ||
_prefix = 'Q' | ||
|
||
def isAnchored(self): | ||
return (self.n == 1 and self.startingMonth is not None) | ||
|
||
@apply_wraps | ||
def apply(self, other): | ||
n = self.n | ||
|
@@ -2017,9 +2000,6 @@ class QuarterBegin(QuarterOffset): | |
_from_name_startingMonth = 1 | ||
_prefix = 'QS' | ||
|
||
def isAnchored(self): | ||
return (self.n == 1 and self.startingMonth is not None) | ||
|
||
@apply_wraps | ||
def apply(self, other): | ||
n = self.n | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole thing should just be moved to a separate function, much more clear this way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you're referring to as "this whole thing".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this formatting function (e.g. repr should just call it, passing parameters in).