BUG: index.getitem raising ValueError with boolean indexer and NA #45985
Conversation
phofl
added
Index
| # time below from 3.8 ms to 496 µs | ||
| # if we already have ndarray[bool], the overhead is 1.4 µs or .25% | ||
| key = np.asarray(key, dtype=bool) | ||
| if is_extension_array_dtype(getattr(key, "dtype", None)): |
There was a problem hiding this comment.
clearer or more performant to do isinstance(key, ExtensionArray)?
There was a problem hiding this comment.
Thought about this too, but does not work unfortunately, since key is a Series here
| # GH#45806 | ||
| ser = pd.Series([True, False, pd.NA], dtype="boolean") | ||
| result = ser.index[ser] | ||
| expected = Index([0]) |
There was a problem hiding this comment.
is there a discussion where it was decided that this is the correct behavior? could plausibly raise.
could also use ser._values or Index(ser) for the key?
There was a problem hiding this comment.
This works for a regular bool series, so thought that this should work too. We can add these test cases too, but if Series should work we have to keep that one
There was a problem hiding this comment.
@phofl just so im clear, your comment is responding to the "could also use..." part of mine? If so that's fine. I'm unclear on the other part of why we're not raising on pd.NA
There was a problem hiding this comment.
Ah now I got you.
Why should we raise on pd.NA?
There was a problem hiding this comment.
Why should we raise on pd.NA?
I think of idx[mask] as akin to [idx[n] for n in range(len(mask)) if mask[n]] which would raise.
3 participants
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.