Skip to content

Pin correct names to wrapped Index comparison methods #18397

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

Merged
merged 2 commits into from
Nov 22, 2017
Merged
Show file tree
Hide file tree
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
5 changes: 3 additions & 2 deletions pandas/core/indexes/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from pandas._libs.lib import is_datetime_array
from pandas._libs.tslibs import parsing

from pandas.compat import range, u
from pandas.compat import range, u, set_function_name
from pandas.compat.numpy import function as nv
from pandas import compat

Expand Down Expand Up @@ -3890,7 +3890,8 @@ def _evaluate_compare(self, other):
except TypeError:
return result

return _evaluate_compare
name = '__{name}__'.format(name=op.__name__)
return set_function_name(_evaluate_compare, name, cls)

cls.__eq__ = _make_compare(operator.eq)
cls.__ne__ = _make_compare(operator.ne)
Expand Down
6 changes: 3 additions & 3 deletions pandas/core/indexes/category.py
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ def _codes_for_groupby(self, sort):
def _add_comparison_methods(cls):
""" add in comparison methods """

def _make_compare(op):
def _make_compare(opname):
def _evaluate_compare(self, other):

# if we have a Categorical type, then must have the same
Expand All @@ -745,9 +745,9 @@ def _evaluate_compare(self, other):
"have the same categories and ordered "
"attributes")

return getattr(self.values, op)(other)
return getattr(self.values, opname)(other)

return _evaluate_compare
return compat.set_function_name(_evaluate_compare, opname, cls)

cls.__eq__ = _make_compare('__eq__')
cls.__ne__ = _make_compare('__ne__')
Expand Down
31 changes: 18 additions & 13 deletions pandas/core/indexes/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def f(self):
return property(f)


def _dt_index_cmp(opname, nat_result=False):
def _dt_index_cmp(opname, cls, nat_result=False):
"""
Wrap comparison operations to convert datetime-like to datetime64
"""
Expand Down Expand Up @@ -135,7 +135,7 @@ def wrapper(self, other):
return result
return Index(result)

return wrapper
return compat.set_function_name(wrapper, opname, cls)


def _ensure_datetime64(other):
Expand Down Expand Up @@ -277,12 +277,15 @@ def _join_i8_wrapper(joinf, **kwargs):
libjoin.left_join_indexer_unique_int64, with_indexers=False)
_arrmap = None

__eq__ = _dt_index_cmp('__eq__')
__ne__ = _dt_index_cmp('__ne__', nat_result=True)
__lt__ = _dt_index_cmp('__lt__')
__gt__ = _dt_index_cmp('__gt__')
__le__ = _dt_index_cmp('__le__')
__ge__ = _dt_index_cmp('__ge__')
@classmethod
def _add_comparison_methods(cls):
""" add in comparison methods """
cls.__eq__ = _dt_index_cmp('__eq__', cls)
cls.__ne__ = _dt_index_cmp('__ne__', cls, nat_result=True)
cls.__lt__ = _dt_index_cmp('__lt__', cls)
cls.__gt__ = _dt_index_cmp('__gt__', cls)
cls.__le__ = _dt_index_cmp('__le__', cls)
cls.__ge__ = _dt_index_cmp('__ge__', cls)

_engine_type = libindex.DatetimeEngine

Expand Down Expand Up @@ -1596,14 +1599,15 @@ def slice_indexer(self, start=None, end=None, step=None, kind=None):
else:
raise

# alias to offset
def _get_freq(self):
@property
def freq(self):
"""get/set the frequency of the Index"""
return self.offset

def _set_freq(self, value):
@freq.setter
def freq(self, value):
"""get/set the frequency of the Index"""
self.offset = value
freq = property(fget=_get_freq, fset=_set_freq,
doc="get/set the frequency of the Index")

year = _field_accessor('year', 'Y', "The year of the datetime")
month = _field_accessor('month', 'M',
Expand Down Expand Up @@ -2011,6 +2015,7 @@ def to_julian_date(self):
) / 24.0)


DatetimeIndex._add_comparison_methods()
DatetimeIndex._add_numeric_methods_disabled()
DatetimeIndex._add_logical_methods_disabled()
DatetimeIndex._add_datetimelike_methods()
Expand Down
21 changes: 13 additions & 8 deletions pandas/core/indexes/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def dt64arr_to_periodarr(data, freq, tz):
_DIFFERENT_FREQ_INDEX = period._DIFFERENT_FREQ_INDEX


def _period_index_cmp(opname, nat_result=False):
def _period_index_cmp(opname, cls, nat_result=False):
"""
Wrap comparison operations to convert datetime-like to datetime64
"""
Expand Down Expand Up @@ -115,7 +115,8 @@ def wrapper(self, other):
result[self._isnan] = nat_result

return result
return wrapper

return compat.set_function_name(wrapper, opname, cls)


def _new_PeriodIndex(cls, **d):
Expand Down Expand Up @@ -227,12 +228,15 @@ class PeriodIndex(DatelikeOps, DatetimeIndexOpsMixin, Int64Index):

_engine_type = libindex.PeriodEngine

__eq__ = _period_index_cmp('__eq__')
__ne__ = _period_index_cmp('__ne__', nat_result=True)
__lt__ = _period_index_cmp('__lt__')
__gt__ = _period_index_cmp('__gt__')
__le__ = _period_index_cmp('__le__')
__ge__ = _period_index_cmp('__ge__')
@classmethod
def _add_comparison_methods(cls):
""" add in comparison methods """
cls.__eq__ = _period_index_cmp('__eq__', cls)
cls.__ne__ = _period_index_cmp('__ne__', cls, nat_result=True)
cls.__lt__ = _period_index_cmp('__lt__', cls)
cls.__gt__ = _period_index_cmp('__gt__', cls)
cls.__le__ = _period_index_cmp('__le__', cls)
cls.__ge__ = _period_index_cmp('__ge__', cls)

def __new__(cls, data=None, ordinal=None, freq=None, start=None, end=None,
periods=None, copy=False, name=None, tz=None, dtype=None,
Expand Down Expand Up @@ -1102,6 +1106,7 @@ def tz_localize(self, tz, infer_dst=False):
raise NotImplementedError("Not yet implemented for PeriodIndex")


PeriodIndex._add_comparison_methods()
PeriodIndex._add_numeric_methods_disabled()
PeriodIndex._add_logical_methods_disabled()
PeriodIndex._add_datetimelike_methods()
Expand Down
20 changes: 12 additions & 8 deletions pandas/core/indexes/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def f(self):
return property(f)


def _td_index_cmp(opname, nat_result=False):
def _td_index_cmp(opname, cls, nat_result=False):
"""
Wrap comparison operations to convert timedelta-like to timedelta64
"""
Expand Down Expand Up @@ -93,7 +93,7 @@ def wrapper(self, other):
return result
return Index(result)

return wrapper
return compat.set_function_name(wrapper, opname, cls)


class TimedeltaIndex(DatetimeIndexOpsMixin, TimelikeOps, Int64Index):
Expand Down Expand Up @@ -180,12 +180,15 @@ def _join_i8_wrapper(joinf, **kwargs):
_datetimelike_methods = ["to_pytimedelta", "total_seconds",
"round", "floor", "ceil"]

__eq__ = _td_index_cmp('__eq__')
__ne__ = _td_index_cmp('__ne__', nat_result=True)
__lt__ = _td_index_cmp('__lt__')
__gt__ = _td_index_cmp('__gt__')
__le__ = _td_index_cmp('__le__')
__ge__ = _td_index_cmp('__ge__')
@classmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we define the _add_comparison_method to take a function and live in indexes/base? to avoid this repeated code?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm open to this. It isn't entirely trivial because it references DatetimeIndex, TimedeltaIndex, super... I'll take a look at how much datetimelike-specific logic is there; it may be more at home in indexes.datetimelike.

For now I'd advocate closing this fairly straightforward fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok pls add to list (opening an issue is ok too, e.g. if its a bigger thing, not likely to get to anytime soon, etc).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yah there's definitely some logic shared between the timedeltas and datetimes version, but its a little bit deceptive in part because _to_m8 is defined differently in each module.

def _add_comparison_methods(cls):
""" add in comparison methods """
cls.__eq__ = _td_index_cmp('__eq__', cls)
cls.__ne__ = _td_index_cmp('__ne__', cls, nat_result=True)
cls.__lt__ = _td_index_cmp('__lt__', cls)
cls.__gt__ = _td_index_cmp('__gt__', cls)
cls.__le__ = _td_index_cmp('__le__', cls)
cls.__ge__ = _td_index_cmp('__ge__', cls)

_engine_type = libindex.TimedeltaEngine

Expand Down Expand Up @@ -912,6 +915,7 @@ def delete(self, loc):
return TimedeltaIndex(new_tds, name=self.name, freq=freq)


TimedeltaIndex._add_comparison_methods()
TimedeltaIndex._add_numeric_methods()
TimedeltaIndex._add_logical_methods_disabled()
TimedeltaIndex._add_datetimelike_methods()
Expand Down
8 changes: 8 additions & 0 deletions pandas/tests/indexes/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -2174,3 +2174,11 @@ class TestIndexUtils(object):
def test_ensure_index_from_sequences(self, data, names, expected):
result = _ensure_index_from_sequences(data, names)
tm.assert_index_equal(result, expected)


@pytest.mark.parametrize('opname', ['eq', 'ne', 'le', 'lt', 'ge', 'gt'])
def test_generated_op_names(opname, indices):
index = indices
opname = '__{name}__'.format(name=opname)
method = getattr(index, opname)
assert method.__name__ == opname