-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: GH12896 where extra elements are returned in MultiIndex slicing #13117
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
Conversation
@@ -1761,7 +1761,7 @@ def convert_indexer(start, stop, step, indexer=indexer, labels=labels): | |||
|
|||
else: | |||
m = np.zeros(len(labels), dtype=bool) | |||
m[np.in1d(labels, r, assume_unique=True)] = True | |||
m[np.in1d(labels, r, assume_unique=False)] = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I think maybe you can set the unique flag based on whether labels is unique no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that what MultiIndex.is_unique
is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC in this case labels
is just an index, but yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assume_unique=labels.is_unique
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you can gin up tests that are both unique & non-unique just to have them together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a FrozenNDArray
. So I will use the MultiIndex
one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh the do Index(labels).is_unique
I think if we pass False always it is much slower
@@ -121,7 +121,7 @@ Bug Fixes | |||
|
|||
|
|||
|
|||
|
|||
- Bug in ``MultiIndex`` slicing where extra elements were returned (:issue:`12896`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when level is non-unique
pls update when you can. |
Will do tonight! |
Current coverage is 84.14%@@ master #13117 diff @@
==========================================
Files 138 137 -1
Lines 50384 50287 -97
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 42382 42312 -70
+ Misses 8002 7975 -27
Partials 0 0
|
ty sir! |
git diff upstream/master | flake8 --diff
Is this the right way to do this?
cc @MaximilianR