Skip to content

BUG: Fix IntervalIndex.get_loc/get_indexer for IntervalIndex of length one #20946

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
May 5, 2018
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
2 changes: 2 additions & 0 deletions doc/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1632,6 +1632,8 @@ IntervalIndex Components
IntervalIndex.length
IntervalIndex.values
IntervalIndex.is_non_overlapping_monotonic
IntervalIndex.get_loc
IntervalIndex.get_indexer


.. _api.multiindex:
Expand Down
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.23.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,7 @@ Indexing
- Bug in ``Series.is_unique`` where extraneous output in stderr is shown if Series contains objects with ``__ne__`` defined (:issue:`20661`)
- Bug in ``.loc`` assignment with a single-element list-like incorrectly assigns as a list (:issue:`19474`)
- Bug in partial string indexing on a ``Series/DataFrame`` with a monotonic decreasing ``DatetimeIndex`` (:issue:`19362`)
- Bug in :meth:`IntervalIndex.get_loc` and :meth:`IntervalIndex.get_indexer` when used with an :class:`IntervalIndex` containing a single interval (:issue:`17284`, :issue:`20921`)

MultiIndex
^^^^^^^^^^
Expand Down
19 changes: 12 additions & 7 deletions pandas/core/indexes/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,22 @@ class IntervalIndex(IntervalMixin, Index):

Attributes
----------
left
right
closed
mid
is_non_overlapping_monotonic
left
length
mid
right
values
is_non_overlapping_monotonic

Methods
-------
contains
from_arrays
from_tuples
from_breaks
contains
from_tuples
get_indexer
get_loc
Copy link
Member Author

Choose a reason for hiding this comment

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

Reorganized here to be alphabetical.


Examples
---------
Expand Down Expand Up @@ -938,8 +940,11 @@ def _searchsorted_monotonic(self, label, side, exclude_label=False):
if isinstance(label, IntervalMixin):
raise NotImplementedError

# GH 20921: "not is_monotonic_increasing" for the second condition
# instead of "is_monotonic_decreasing" to account for single element
# indexes being both increasing and decreasing
if ((side == 'left' and self.left.is_monotonic_increasing) or
(side == 'right' and self.left.is_monotonic_decreasing)):
(side == 'right' and not self.left.is_monotonic_increasing)):
sub_idx = self.right
if self.open_right or exclude_label:
label = _get_next_label(label)
Expand Down
18 changes: 18 additions & 0 deletions pandas/tests/indexes/interval/test_interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,14 @@ def test_get_loc_interval(self):
pytest.raises(KeyError, self.index.get_loc,
Interval(-1, 0, 'left'))

# Make consistent with test_interval_new.py (see #16316, #16386)
@pytest.mark.parametrize('item', [3, Interval(1, 4)])
def test_get_loc_length_one(self, item, closed):
# GH 20921
index = IntervalIndex.from_tuples([(0, 5)], closed=closed)
result = index.get_loc(item)
assert result == 0

# To be removed, replaced by test_interval_new.py (see #16316, #16386)
def test_get_indexer(self):
actual = self.index.get_indexer([-1, 0, 0.5, 1, 1.5, 2, 3])
Expand Down Expand Up @@ -544,6 +552,16 @@ def test_get_indexer_subintervals(self):
expected = np.array([0, 0, 0], dtype='intp')
tm.assert_numpy_array_equal(actual, expected)

# Make consistent with test_interval_new.py (see #16316, #16386)
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is consistent with those tests, you can remove from there I think (not sure exactly how we are tracking 'new' things vs current)

Copy link
Member Author

@jschendel jschendel May 4, 2018

Choose a reason for hiding this comment

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

This isn't consistent with the new tests, as the new behavior should only allow for exact matches when querying with Interval objects. Mostly added this a a note to whoever implements the new behavior (probably me).

I've taken a stab at implementing the new behavior in the past, but it looks like it will require a pretty large change, as changing one method to implement the new behavior often causes another (also with new proposed behavior) to break, so it seems like it'd at least approach an all or nothing type of change. Hopefully will carve out some time to do it fully somewhat soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

k that is fine. yeah converting to use the new tests is a major effort :>

@pytest.mark.parametrize('item', [
[3], np.arange(1, 5), [Interval(1, 4)], interval_range(1, 4)])
def test_get_indexer_length_one(self, item, closed):
# GH 17284
index = IntervalIndex.from_tuples([(0, 5)], closed=closed)
result = index.get_indexer(item)
expected = np.array([0] * len(item), dtype='intp')
tm.assert_numpy_array_equal(result, expected)

# To be removed, replaced by test_interval_new.py (see #16316, #16386)
def test_contains(self):
# Only endpoints are valid.
Expand Down