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: GH12896 where extra elements are returned in MultiIndex slicing #13117

Closed
wants to merge 1 commit into from

Conversation

kawochen
Copy link
Contributor

@kawochen kawochen commented May 8, 2016

Is this the right way to do this?

cc @MaximilianR

@@ -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
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor

@jreback jreback May 8, 2016

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

Copy link
Contributor Author

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?

Copy link
Contributor

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

@jreback jreback added Bug Indexing Related to indexing on series/frames, not to indexes themselves MultiIndex labels May 8, 2016
@@ -121,7 +121,7 @@ Bug Fixes




- Bug in ``MultiIndex`` slicing where extra elements were returned (:issue:`12896`)
Copy link
Contributor

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

@jreback
Copy link
Contributor

jreback commented May 11, 2016

pls update when you can.

@jreback jreback added this to the 0.18.2 milestone May 11, 2016
@kawochen
Copy link
Contributor Author

Will do tonight!

@codecov-io
Copy link

codecov-io commented May 14, 2016

Current coverage is 84.14%

Merging #13117 into master will increase coverage by +<.01%

@@             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          
  1. 2 files (not in diff) in pandas/io were modified. more
    • Misses +57
    • Hits +3
  2. 5 files (not in diff) in pandas/core were modified. more
    • Misses -16
    • Hits -69
  3. 4 files (not in diff) in pandas were modified. more
    • Misses -2
    • Hits -5
  4. File ...das/indexes/multi.py was modified. more
    • Misses 0
    • Partials 0
    • Hits +1
  5. File pandas/io/s3.py (not in diff) was deleted. more

Powered by Codecov. Last updated by 01dd111...94a6bed

@jreback
Copy link
Contributor

jreback commented May 14, 2016

ty sir!

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 MultiIndex
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiindex slicing df.loc[idx[dim1,dim2,dim3],:] not working right in some cases
3 participants