BUG: Fix IntervalArray equality comparisions #30640
Conversation
| @@ -547,6 +550,58 @@ def __setitem__(self, key, value): | |||
| right.values[key] = value_right | |||
| self._right = right | |||
|
|
|||
| def __eq__(self, other): | |||
| # ensure pandas array for list-like and eliminate non-interval scalars | |||
| if is_list_like(other): | |||
There was a problem hiding this comment.
this particular check can be pushed to the base class
There was a problem hiding this comment.
I don't quite follow. Whose the base class here? ExtensionArray?
| return np.zeros(len(self), dtype=bool) | ||
|
|
||
| # determine the dtype of the elements we want to compare | ||
| if isinstance(other, Interval): |
There was a problem hiding this comment.
at this point can’t u just wrap other in array()?
| @@ -547,6 +550,58 @@ def __setitem__(self, key, value): | |||
| right.values[key] = value_right | |||
| self._right = right | |||
|
|
|||
| def __eq__(self, other): | |||
| # ensure pandas array for list-like and eliminate non-interval scalars | |||
| if is_list_like(other): | |||
There was a problem hiding this comment.
I don't quite follow. Whose the base class here? ExtensionArray?
| "overlaps", | ||
| "contains", | ||
| "__eq__", | ||
| "__ne__", |
There was a problem hiding this comment.
to be consistent with our other EAs, these need to be dispatched/wrapped the same way they are in datetimelike or categorical. im planning to move the relevant code to indexes.extension so this can re-use the existing code.
The relevant test will be arr == series --> Series, idx == series --> ndarray[bool]
There was a problem hiding this comment.
You should now be able to use indexes.extension.make_wrapped_comparison_op
| @@ -93,6 +113,221 @@ def test_set_na(self, left_right_dtypes): | |||
| tm.assert_extension_array_equal(result, expected) | |||
|
|
|||
|
|
|||
| class TestComparison: | |||
There was a problem hiding this comment.
possibly put in tests.arithmetic.test_interval and parametrize with box_with_array
|
@jschendel ok, hopefully some of this can be pushed down to base classes; @jbrockmendel merge if your comments have been addressed. |
| isinstance(obj, Interval) | ||
| and self.closed == obj.closed | ||
| and self.left[i] == obj.left | ||
| and self.right[i] == obj.right |
There was a problem hiding this comment.
can this check be just self[i] == obj?
There was a problem hiding this comment.
It could but there'd be a perf hit for actually materializing the Interval object.
Some of this probably could be pushed down to base classes but need to be careful because it could cause a perf hit. In the |
|
ok fIr @jschendel yep we often balance perf of specialized code paths vs more generic code |
|
k thanks @jschendel |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffcc @TomAugspurger