Skip to content
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

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

Merged
merged 2 commits into from May 5, 2018

Conversation

jschendel
Copy link
Member

@codecov
Copy link

codecov bot commented May 4, 2018

Codecov Report

Merging #20946 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #20946      +/-   ##
==========================================
- Coverage   91.81%   91.81%   -0.01%     
==========================================
  Files         153      153              
  Lines       49479    49478       -1     
==========================================
- Hits        45428    45427       -1     
  Misses       4051     4051
Flag Coverage Δ
#multiple 90.2% <ø> (-0.01%) ⬇️
#single 41.85% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/interval.py 93.08% <ø> (ø) ⬆️
pandas/core/strings.py 98.62% <0%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28dbae9...3c82ef3. Read the comment docs.

@jreback jreback added Bug Indexing Related to indexing on series/frames, not to indexes themselves Interval Interval data type labels May 4, 2018
@@ -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 ``IntervalIndex.get_loc()`` and ``IntervalIndex.get_indexer()`` when used with an :class:`IntervalIndex` containing a single interval (:issue:`17284`, :issue:`20921`)
Copy link
Contributor

Choose a reason for hiding this comment

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

are these i the api.rst? if not can you can add and put refs here?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -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 :>

@jreback jreback added this to the 0.23.0 milestone May 4, 2018
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.

@jreback jreback merged commit e8e6e89 into pandas-dev:master May 5, 2018
@jreback
Copy link
Contributor

jreback commented May 5, 2018

thanks @jschendel. always nice to have a great patch from you!

@jschendel jschendel deleted the ii-get-loc-single branch May 5, 2018 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Indexing Related to indexing on series/frames, not to indexes themselves Interval Interval data type
Projects
None yet
2 participants