Skip to content

More speedups for Period comparisons #21606

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jun 26, 2018
Merged

More speedups for Period comparisons #21606

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9138150
cache harder
jbrockmendel Jun 23, 2018
f2fff9e
Move params implementation to cython
jbrockmendel Jun 25, 2018
fa547e6
Merge branch 'master' of https://github.com/pandas-dev/pandas into pc…
jbrockmendel Jun 25, 2018
eb69dd3
update whatsnew
jbrockmendel Jun 25, 2018
04c8e89
typo fixup
jbrockmendel Jun 25, 2018
7d75ef4
Merge branch 'master' of https://github.com/pandas-dev/pandas into pc…
jbrockmendel Jun 26, 2018
7c9b121
comment, whitespace
jbrockmendel Jun 26, 2018
92e6fdd
implement to_offset
jbrockmendel Jun 26, 2018
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
24 changes: 24 additions & 0 deletions pandas/_libs/tslibs/offsets.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,30 @@ class _BaseOffset(object):
def __setattr__(self, name, value):
raise AttributeError("DateOffset objects are immutable.")

def __eq__(self, other):
if is_string_object(other):
from pandas.tseries.frequencies import to_offset
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you want to define a function to return this import, we use this in a number of places?

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't this just be trading one one-liner for another?

Copy link
Contributor

Choose a reason for hiding this comment

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

no then then its a function which we can import at the top (and of course if we eventually are able to actually move to_offset to cython its a big win). but its 'cleaner' I think. (certainly can followup on this in new PR). I just don't really like the imports hidden in the functions (as we must do currently), and rather centrailize in a single place.

other = to_offset(other)

try:
return self._params == other._params
except AttributeError:
# other is not a DateOffset object
return False

return self._params == other._params

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

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

@property
def _params(self):
from pandas.errors import AbstractMethodError
Copy link
Contributor

Choose a reason for hiding this comment

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

can you import this at the top?

raise AbstractMethodError(self)

@property
def kwds(self):
# for backwards-compatibility
Expand Down
18 changes: 0 additions & 18 deletions pandas/tseries/offsets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,24 +322,6 @@ def _repr_attrs(self):
def name(self):
return self.rule_code

def __eq__(self, other):

if isinstance(other, compat.string_types):
from pandas.tseries.frequencies import to_offset

other = to_offset(other)

if not isinstance(other, DateOffset):
return False

return self._params == other._params

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

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

def __add__(self, other):
if isinstance(other, (ABCDatetimeIndex, ABCSeries)):
return other + self
Expand Down