-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
ENH: Support Series[bool] as indexer for iloc.__getitem__
#61162
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
ENH: Support Series[bool] as indexer for iloc.__getitem__
#61162
Conversation
iloc.__getitem__
iloc.__getitem__iloc.__getitem__
iloc.__getitem__iloc.__getitem__
rhshadrach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Am I correct that none of the tests being modified here actually call .iloc successfully?
pandas/tests/indexing/test_iloc.py
Outdated
| mask = (df.nums > 2).values | ||
| if idx: | ||
| mask_index = getattr(df, idx)[::-1] | ||
| mask_index = getattr(df, idx if idx == "index" else "locs")[::-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since idx is not None here, I think this can be reverted.
pandas/tests/indexing/test_iloc.py
Outdated
| except ( | ||
| ValueError, | ||
| IndexingError, | ||
| ) as err: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can you collapse this back to one line
|
Hi! The test on lines 742-743 calls result = df.iloc[np.array([True] * len(mask), dtype=bool)]
tm.assert_frame_equal(result, df)I noticed I should add another test with false values so I have done so. result2 = df.iloc[np.array([True, False, True, False, True], dtype=bool)]
tm.assert_frame_equal(result2, DataFrame({"a": [0, 2, 4]}, index=["A", "C", "E"])) |
|
While reviewing this, I realized my tests address That being said the new tests improve coverage for |
iloc.__getitem__Series[bool] as indexer for iloc.__getitem__
rhshadrach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; @mroeschke - could I get your eye here as well.
pandas/tests/indexing/test_iloc.py
Outdated
| result = df.iloc[np.array([True, False, True, False, True], dtype=bool)] | ||
| tm.assert_frame_equal( | ||
| result, DataFrame({"a": [0, 2, 4]}, index=["A", "C", "E"]) | ||
| ) | ||
|
|
||
| # series (index does not match) | ||
| msg = "Unalignable boolean Series provided as indexer" | ||
| with pytest.raises(IndexingError, match=msg): | ||
| df.iloc[Series([True] * len(mask), dtype=bool)] | ||
|
|
||
| df = DataFrame(list(range(5)), columns=["a"]) | ||
|
|
||
| result = df.iloc[Series([True] * len(mask), dtype=bool)] | ||
| tm.assert_frame_equal(result, df) | ||
|
|
||
| result = df.iloc[Series([True, False, True, False, True], dtype=bool)] | ||
| tm.assert_frame_equal(result, DataFrame({"a": [0, 2, 4]}, index=[0, 2, 4])) | ||
|
|
||
| df = DataFrame(list(range(5)), index=list("ABCDE"), columns=["a"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make each of these assertions a separate test function?
|
Thanks @arthurlw |
…-dev#61162) * updated indexing.py to allow iloc.__getitem__ * Updated test_iloc_mask test * bugfix test_iloc_mask test * bugfix test_iloc_mask * whatsnew * added test to test_iloc_mask * formatting * precommit * added tests for series bool mask * precommit * reformatted tests
ilocwithSeriesas indexer fails for__getitem__but works with__setitem__#60994doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.