ENH: Enable indexing with nullable Boolean
State: closed

closes #31503 

- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

I think from the discussion in https://github.com/pandas-dev/pandas/issues/31503 that this is something people want to allow.

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: NA - MaskedArrays
description: Related to pd.NA and nullable extension arrays

Comments:
so since indexing for these's EAs is experimental, I would be ok with doing this for 1.0.2.

@TomAugspurger @jorisvandenbossche 
So the discussion I am having with @dsaxton in the inline comments in the following:

Assume we do boolean indexing with a nullable boolean array (or series):

```
s = pd.Series(range(3))
s[pd.array([True, False, pd.NA], dtype="boolean")]   
```

then for consistency, we typically allow to do the same with a list as well:

```
s[[True, False, pd.NA]]
```

(at least that is how indexing now works assuming it would only be True/False values in the list)

However, the question is then: how do we convert such a list to an array? We have `pd.array` for this, but then something like `s[[True, False, None]]` will also be interpreted as nullable booleans, while now this would be an object dtype that will raise an error because you can't index with nulls (and the same for `s[[True, False, np.nan]]`). 

This is because `pd.array([True, False, None])` will infer a boolean array with missing values. 

So:

* This would be a change in behaviour, since indexing with `[True, False, None]` was already possible before the introduction of `pd.NA` and raised an error (although I think we can consider this "mask as list" as a corner case).
* This also interprets None and np.nan as pd.NA, while pd.NA has a different behaviour in those indexing/boolean situations.

Are we OK with that? Or would we like to have a kind of "strict" list conversion, where only actual pd.NA values are seen as missing values?

(whether `pd.array([True, False, np.nan])` should infer boolean and "silently" converts np.nan into pd.NA can maybe also be a discussion)



(and BTW, I think my comment above discussing this "list" case is rather a corner case, and so I think we can also leave that out for this PR and have that discussion separately, if that makes things easier)
should not have any special cases in boolean indexing

let’s just go back to the any null value is simply filled False and call it a day
it’s very easy to understand and just works with no downsides


We already have a special case in boolean indexing, which is that we allow object dtype with boolean values, but raise if those contain missing values. We are not going to change that in this PR. 

The question is how to interpret a list of bools + None/np.nan. Continue interpreting this as object dtype? (and thus raising an error) Or coercing this to nullable  boolean array? (thus changing the indexing behaviour as this PR is introducing)
I think just coerce to a nullable boolean array. The rule "list-like inputs are cast with `pd.array(key)` is easy to explain.
A possible counterargument could be that we'd be making changes to a non-experimental feature (masking with a list of booleans and potentially null values) for the sake of changing an experimental one (masking with a BooleanArray). Also, if we feel that masking with a list is a corner case, that could be seen as an argument for leaving `[True, False, pd.NA]` as-is (even if it's slightly inconsistent with `pd.array([True, False, pd.NA])`), rather than changing the behavior of list masks universally.

I'm mostly concerned because I'm seeing some surprising test failures after making this change (e.g., somehow this of all lines raises: https://github.com/pandas-dev/pandas/blob/master/pandas/tests/indexing/test_loc.py#L968), probably because `is_bool_indexer` is used in a lot of places (or perhaps there's a safer way to make the change).
As I said before, I am fine with leaving out fixing the list case for now
Just to confirm, the expected behavior for setitem is to treat NAs as False?
> BTW, we also need to update setitem then, in addition to getitem

It looks like `setitem` might just work now after updating `check_bool_indexer` but will add some tests to try to confirm
> Just to confirm, the expected behavior for setitem is to treat NAs as False?

Yes, I would assume so.

> will add some tests to try to confirm

There was just a PR merged that added a bunch of setitem tests, so if you merge latest master, you can update those
> There was just a PR merged that added a bunch of setitem tests, so if you merge latest master, you can update those

Included a case that replaces False with pd.NA in the setitem tests just added. I think it's safe to simply remove the ones that test only for an error message?
> > There was just a PR merged that added a bunch of setitem tests, so if you merge latest master, you can update those
> 
> Included a case that replaces False with pd.NA in the setitem tests just added. I think it's safe to simply remove the ones that test only for an error message?

no, simply re-write the tests rather than removing (as indicated)
@jreback Seems to be green, the CI failures are due to an apparent web doc issue: https://github.com/pandas-dev/pandas/issues/32060
@jorisvandenbossche It seems that `check_array_indexer` works okay with sparse input, so maybe the comment is outdated?

```python
In [1]: import numpy as np 
   ...: import pandas as pd 
   ...: from pandas.core.indexing import check_bool_indexer 
   ...: from pandas.core.indexers import check_array_indexer 
   ...:  
   ...: arr = pd.arrays.SparseArray([True, False]) 
   ...: idx = np.array([1, 2]) 
   ...:  
   ...: check_array_indexer(idx, arr) 
   ...:                                                                                                                                                                            
Out[1]: array([ True, False])
```

> It seems that check_array_indexer works okay with sparse input, so maybe the comment is outdated?

That seems so then, yes. Last question: can you update the comment then?
thanks @dsaxton 

very nice!
@dsaxton 

hmm not sure why this didn’t backport automatically
can u push a PR
> @dsaxton
> 
> hmm not sure why this didn’t backport automatically
> can u push a PR

I'm not too familiar with how backporting works, do I just put a PR against the `1.0.x` branch instead of `master`?
yes checkout the 1.0.x and cherry pick the commit then push to a new remote branch but before you do that let me see if i can get the bot to do it
@Meeseeksdev backport to 1.0.x
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
$ git checkout 1.0.x
$ git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
$ git cherry-pick -m1 b9bcdc30765e88718c792b27ab9f3e27054f9fc7
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
$ git commit -am 'Backport PR #31591: ENH: Enable indexing with nullable Boolean'
```

4. Push to a named branch :

```
git push YOURFORK 1.0.x:auto-backport-of-pr-31591-on-1.0.x
```

5. Create a PR against branch 1.0.x, I would have named this PR:

> "Backport PR #31591 on branch 1.0.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
                
@dsaxton ok instructions above 
> @dsaxton ok instructions above

Created this PR: https://github.com/pandas-dev/pandas/pull/32192