BUG: Regression in .loc accepting a boolean Index as an indexer

[jreback]

closes #17131

[codecov]

Codecov Report

Merging #17738 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17738      +/-   ##
==========================================
- Coverage   91.22%    91.2%   -0.02%     
==========================================
  Files         163      163              
  Lines       49813    49813              
==========================================
- Hits        45442    45433       -9     
- Misses       4371     4380       +9

Flag	Coverage Δ
#multiple	89% <100%> (ø)	⬆️
#single	40.33% <100%> (-0.06%)	⬇️

Impacted Files	Coverage Δ
pandas/core/common.py	91.42% <100%> (ø)	⬆️
pandas/io/gbq.py	25% <0%> (-58.34%)	⬇️
pandas/core/frame.py	97.73% <0%> (-0.1%)	⬇️

---

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 bf5b089...1d63701. Read the comment docs.

[jorisvandenbossche]

@jreback This is not a regression bug fix, but an backwards incompatible API change.

Not saying that we shouldn't do this, but that we have to take a bit more care to think about it (and possibly deprecate, although given that the previous behaviour of using it as labels was broken in the latest release, maybe not needed), and certainly note it in the whatsnew as such.

[jorisvandenbossche]

Actually, it was only broken in master, not in the latest release.

So to clarify what I mean, before the boolean values were interpreted as labels:

In [53]: pd.Series([1, 2, 3]).loc[pd.Index([True, False, True])]
Out[53]: 
1    2
0    1
1    2
dtype: int64


In [54]: pd.__version__
Out[54]: '0.20.3'

which is not useful, so I agree changing this is a good idea.
But the question is then what to do with booleans in the index? Shouldn't this be kept label-based rather than boolean mask ?

[jreback]

@jorisvandenbossche oh I though this was broken in 0.20.3; its a regression none the less.

your example is unambiguous. an Index of booleans is de-facto the same as a boolean ndarray, it has no other interpretation that as a boolean indexer (a boolean Series is completely different of course).

I'll remove the whatsnew note then.

[jreback]

I think some refactoring broke this (and was not explicity tested).

[jreback]

@jorisvandenbossche actually looking at your example [53] again, I think that is completely broken and not expected behavior at all. A boolean indexer by-definition is not label based. I guess could add a note about that.

[jreback]

@jorisvandenbossche actually, that is even moar broken [53]. Its interpreting the bools as labels, again completely wrong.

[jorisvandenbossche]

As I said, I agree the behaviour was kind of 'broken' (or at least strange) and good to fix, but still I wouldn't call this a regression fix (it has been like that for a long time). Although I can agree calling it a 'bug fix' for the integer case.

But for the boolean index case it is really a change in behaviour, because there the original behaviour made some sense:

In [58]: pd.__version__
Out[58]: '0.20.3'

In [59]: pd.Series([1, 2, 3], index=[False, True, False]).loc[pd.Index([True, False, True])]
Out[59]: 
True     2
False    1
False    3
True     2
dtype: int64

In [2]: pd.__version__
Out[2]: '0.21.0.dev+560.ga607872'

In [3]: pd.Series([1, 2, 3], index=[False, True, False]).loc[pd.Index([True, False, True])]
Out[3]: 
False    1
False    3
dtype: int64

[jreback]

@jorisvandenbossche ok I'll add a note. We don't have first class support for boolean index anyhow (esp when its duplicated its really weird).

[jorisvandenbossche]

But we could also keep the behaviour (so not use boolean indexing when the index (inferred) type is bool)

[jreback]

But we could also keep the behaviour (so not use boolean indexing when the index (inferred) type is bool)

this is very awkward, an Index is virtually a ndarray except works with all types, for it not to work 'as-expected' in indexing would be odd (and enabling instead for a special case of selecting out a non-supported boolean Index with True/False values)