COMPAT: catch InvalidIndexError in base Indexer getitem #27259
Conversation
| @@ -118,7 +118,7 @@ def __getitem__(self, key): | |||
| key = tuple(com.apply_if_callable(x, self.obj) for x in key) | |||
| try: | |||
| values = self.obj._get_value(*key) | |||
| except (KeyError, TypeError): | |||
| except (KeyError, TypeError, InvalidIndexError): | |||
There was a problem hiding this comment.
can you add a comment about why this is caught here
There was a problem hiding this comment.
Do we need to test this somehow as well?
There was a problem hiding this comment.
See #27258 for a discussion on that. I can add a test with the code snippet I show there, but is a rather dummy example ..
There was a problem hiding this comment.
I guess this is fine, though why are you not using .loc and using __getitem__?
There was a problem hiding this comment.
why are you not using
.locand using__getitem__?
This is from a custom Indexer attribute in geopandas, so it is on purpose not using .loc. Now, I don't say it is the best implementation by using such pandas internals, opened an issue about that in geopandas geopandas/geopandas#1042
gfyoung
added
Dependencies
| @@ -118,7 +118,7 @@ def __getitem__(self, key): | |||
| key = tuple(com.apply_if_callable(x, self.obj) for x in key) | |||
| try: | |||
| values = self.obj._get_value(*key) | |||
| except (KeyError, TypeError): | |||
| except (KeyError, TypeError, InvalidIndexError): | |||
There was a problem hiding this comment.
I guess this is fine, though why are you not using .loc and using __getitem__?
|
thanks @jorisvandenbossche though would avoid using the internals in geopandas :-> |
|
I'd still like a comment here about why we are now catching InvalidIndexerError. Until recently this was catching Exception and I tracked down what actually gets raised, would prefer not to have to do that again in a few months. |
|
@jbrockmendel yes, fully agree, was planning to do that so will do a follow up PR |
Closes #27258