Warn on boolean frame indexer

[gooney47]

• closes BUG: IndexError: positional indexers are out-of-bounds iloc boolean indexing #39004

• tests added / passed

• Ensure all linting tests pass, see here for how to run them

• whatsnew entry

• Is it ok to check index/column to allow things like df[df > 0] or should I just warn on DataFrame in general?

• Can I do the same thing for setitem as well?

Current version warns in 3 already existing tests:

• pandas/tests/frame/indexing/test_where.py::TestDataFrameIndexingWhere::test_where_datetime
• pandas/tests/frame/test_nonunique_indexes.py::TestDataFrameNonuniqueIndexes::test_getitem_boolean_frame_unaligned_with_duplicate_columns
• pandas/tests/indexing/test_indexing.py::TestMisc::test_partial_boolean_frame_indexing

[phofl]

I am not sure is equals is the right thing to do here.

idx = Index([1, 2, 3])

idx2 = Index([3, 2, 1])

idx.equals(idx2)

This returns False but we can certainly align bot indexes

[gooney47]

I agree that they are alignable. I thought the idea was to not accept alignables anymore. I just wanted to make an exception were alignables are accepted if they are already aligned, so that stuff like df[df > 0] is still possible. For every unaligned boolean frame indexer the warning refers to where.

[phofl]

If we are doing this we should be consistent here. Meaning setitem and getitem for Frame and Series.

Additionally we have to discuss what to do with the rhs in setitem

df = DataFrame({"a": [1, 2, 3], "b": [4, 5, 6]})

rhs = DataFrame({"b": [7, 8, 9]}, index=Index([1, 2, 3]))
df[["b"]] = rhs
rhs = DataFrame({"c": [70, 80, 90]}, index=Index([1, 2, 3]))
df[["b"]] = rhs

Both cases currently align on index, but ignore the column.

Edit: Additionally we should document the differences between loc and []. #37516

[jreback]

we merged #39403

does this cover this use case?

[phofl]

No, this fixed a bug but did not touch alignment of missing labels

[jreback]

thanks @phofl

@gooney47 if you can revise according the comments and merge master

[gooney47]

@jreback There is no point for me to continue anything if there is no consensus. I want to warn on unaligned indexers (with indexer I mean boolean DataFrame), @phofl does not want to do this because unaligned indexers can be aligned (?) and you want to warn on any indexer. What do I implement now?

This is also what I asked in the original pull request as a question:

Is it ok to check index/column to allow things like df[df > 0] or should I just warn on DataFrame in general?

[jreback]

@jreback There is no point for me to continue anything if there is no consensus. I want to warn on unaligned indexers (with indexer I mean boolean DataFrame), @phofl does not want to do this because unaligned indexers can be aligned (?) and you want to warn on any indexer. What do I implement now?

This is also what I asked in the original pull request as a question:

Is it ok to check index/column to allow things like df[df > 0] or should I just warn on DataFrame in general?

@gooney47 @phofl was fixing a bug. this is a separate issue. I am ok with warning on a dataframe that that is a boolean array, e.g. needs alignment or not. your original example should show the warning. it is possible that other things may break and need fixing.

[phofl]

Warning in all cases sounds good to me too.

[gooney47]

What to do with tests that throw the new warning?

There are getitem tests that throw the new warning. The question is if they test actual behavior of getitem or of where. If we are lucky there is no actual behavior of getitem in the case of a DataFrame, which would mean all these tests actually are proper where tests.

[gooney47]

Also there might be tests that throw this warning, that are only using getitem/where to test some other functionality in which case there is no need to rename the test.

[jreback]

can you test for setitem as well; also would need to catch anywhere we are actually using it now (or rather simply change those tests)

[gooney47]

I will change the triggering tests on the weekend

[gooney47]

I feel like I made the world a worse place. These changes do not feel good to me, especially because the setitem/getitem indexing feels much more intuitive, than using where and mask. Theoretically you could only use one of them (where or mask), but then you would have to deal with negation of the mask which obfuscates things. If you're on the other hand using both you have to learn two new functions, while people already are familiar with getitem and setitem from all kinds of other objects like lists or numpy arrays. To give example in code: df[mask] translates to either df.where(mask) or df.mask(~mask), while df[mask] = val tanslates to either df.mask(mask, val, True) or df.where(~mask, val, True).

[jreback]

@gooney47

ok i think maybe i pushed you in the wrong direction here. we only want to warn when .iloc is used and a DataFrame indexer. using getitem / .loc are a-ok because these by-definition do alignment (and they are of course convenient). Note we should do this for both .iloc setting and getting.

I think this should significantly narrow the cases where we warn.

[gooney47]

@jreback Ah ok, we had this already here: #39022

[jreback]

@jreback Ah ok, we had this already here: #39022

ok let's talk about this case there.