REF: de-nest Series getitem/setitem checks #33447
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -897,26 +908,14 @@ def __getitem__(self, key): | |||
| else: | |||
| raise | |||
|
|
|||
| if not key_is_scalar: | |||
| # avoid expensive checks if we know we have a scalar | |||
There was a problem hiding this comment.
This comment might be relevant? Also if you move it up as you did now, you can keep this scalar check?
yah, im torn on this. I think following #33404 i can improve the try/except blocks to allow us to retain the current ordering while also getting the de-nesting we get here |
jreback
added
Clean
Indexing
|
can you rebase following #33404 merge with any changes you are planning |
|
mothballing until after the next pass |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ATM the first checks we do in
Series.__getitem__and__setitem__are forget_locwith some not-totally-straightforward try/except logic.This moves the slice case and the com.is_bool_indexer case up, and aligns the ordering of the checks between getitem/setitem.
Getting the com.is_bool_indexer case out from the try/except in setitem is particularly nice.
That said, this may not be what we want to do. This will very slightly improve performance in the cases moved up, and equally slightly harm performance in the scalar case. AFAIK the current ordering is not based on optimizing one case over the others.