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.

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 2 commits into from
Nov 22, 2017
Merged

Pin correct names to wrapped Index comparison methods #18397

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efcef78
Pin correct names to wrapped Index comparison methods
jbrockmendel Nov 20, 2017
c36e870
use set_function_name pattern
jbrockmendel Nov 21, 2017
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
1 change: 1 addition & 0 deletions pandas/core/indexes/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3890,6 +3890,7 @@ def _evaluate_compare(self, other):
except TypeError:
return result

_evaluate_compare.__name__ = '__{name}__'.format(name=op.__name__)
Copy link
Contributor

Choose a reason for hiding this comment

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

we have a function in core/generic for exactly this

(and sets properly in py2/3 and such)

Copy link
Member Author

Choose a reason for hiding this comment

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

Closest I can think of is pd.compat.bind_method. Is that what you're referring to? Wrapped method names are set like this in e.g. indexes.datetimes._field_accessor.

Copy link
Contributor

Choose a reason for hiding this comment

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

use pandas.compat.set_function_name

Copy link
Member Author

Choose a reason for hiding this comment

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

That's going to get into a circularity problem. set_function_name requires cls, but in several cases this is being called before the class has been defined in the namespace.

Scroll up to line 50 in timedeltas for an example of the status quo usage.

Copy link
Contributor

Choose a reason for hiding this comment

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

see how its done in generic.py, instead you should change these to a factor function.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I'll do it, but under protest... those classmethods to _add_foo_methods are among my least favorite design patterns.

Copy link
Contributor

Choose a reason for hiding this comment

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

these are already a well established pattern

and they handle the edge cases

eg qualname

Copy link
Member Author

Choose a reason for hiding this comment

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

In indexes.base.Index the func.__name__ = name pattern is used in _add_logical_methods, _add_logical_methods_disabled, _add_numeric_methods_disabled, _add_numeric_methods_add_sub_disabled. In case you want to open an issue to change those.

return _evaluate_compare

cls.__eq__ = _make_compare(operator.eq)
Expand Down
1 change: 1 addition & 0 deletions pandas/core/indexes/category.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,7 @@ def _evaluate_compare(self, other):

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

_evaluate_compare.__name__ = op
return _evaluate_compare

cls.__eq__ = _make_compare('__eq__')
Expand Down
12 changes: 7 additions & 5 deletions pandas/core/indexes/datetimes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ def wrapper(self, other):
return result
return Index(result)

wrapper.__name__ = opname
return wrapper


Expand Down Expand Up @@ -1596,14 +1597,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
2 changes: 2 additions & 0 deletions pandas/core/indexes/period.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ def wrapper(self, other):
result[self._isnan] = nat_result

return result

wrapper.__name__ = opname
return wrapper


Expand Down
1 change: 1 addition & 0 deletions pandas/core/indexes/timedeltas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def wrapper(self, other):
return result
return Index(result)

wrapper.__name__ = opname
return wrapper


Expand Down
10 changes: 10 additions & 0 deletions pandas/tests/indexes/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2174,3 +2174,13 @@ 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)


def test_generated_op_names(indices):
index = indices
assert index.__eq__.__name__ == '__eq__'
assert index.__ne__.__name__ == '__ne__'
assert index.__le__.__name__ == '__le__'
Copy link
Contributor

Choose a reason for hiding this comment

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

parametrize

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I guess.

assert index.__lt__.__name__ == '__lt__'
assert index.__ge__.__name__ == '__ge__'
assert index.__gt__.__name__ == '__gt__'