Skip to content

PERF: Make DateOffset (partially) immutable, cache _params for __eq__ speedup #17137

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

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from dateutil.relativedelta import relativedelta, weekday
from dateutil.easter import easter
from pandas._libs import tslib, Timestamp, OutOfBoundsDatetime, Timedelta
from pandas.util import cache_readonly

import functools
import operator
Expand Down Expand Up @@ -184,10 +185,17 @@ def __add__(date):
)
_use_relativedelta = False
_adjust_dst = False
_typ = "dateoffset"

# default for prior pickles
normalize = False

def __setattr__(self, key, value):
if key in ['n', '_offset'] and hasattr(self, key):
raise TypeError('%s is intended to be immutable; "%s" '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test to hit this TypeError.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually. First there are some lingering corner cases in which the current implementation can be incorrect.

'cannot be changed.' % (self.__class__, key))
object.__setattr__(self, key, value)

def __init__(self, n=1, normalize=False, **kwds):
self.n = int(n)
self.normalize = normalize
Expand Down Expand Up @@ -308,6 +316,7 @@ def copy(self):
def _should_cache(self):
return self.isAnchored() and self._cacheable

@cache_readonly
def _params(self):
all_paras = dict(list(vars(self).items()) + list(self.kwds.items()))
if 'holidays' in all_paras and not all_paras['holidays']:
Expand Down Expand Up @@ -369,13 +378,13 @@ def __eq__(self, other):
if not isinstance(other, DateOffset):
return False

return self._params() == other._params()
return self._params == other._params

def __ne__(self, other):
return not self == other

def __hash__(self):
return hash(self._params())
return hash(self._params)

def __call__(self, other):
return self.apply(other)
Expand Down Expand Up @@ -808,7 +817,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)
Expand Down Expand Up @@ -2728,7 +2737,7 @@ def __eq__(self, other):
# This is identical to DateOffset.__hash__, but has to be redefined here
# for Python 3, because we've redefined __eq__.
def __hash__(self):
return hash(self._params())
return hash(self._params)

def __ne__(self, other):
if isinstance(other, compat.string_types):
Expand Down