CLN: assorted indexing-related cleanups #31797
Conversation
jbrockmendel
commented
Feb 7, 2020
jbrockmendel
commented
- _setitem_with_indexer isnt consistent about whether or not it returns anything, make it always-None
- avoid using private loc method from DataFrame
| @@ -862,7 +862,9 @@ def __getitem__(self, key): | |||
|
|
|||
| return result | |||
| except InvalidIndexError: | |||
| pass | |||
| if not isinstance(self.index, MultiIndex): | |||
There was a problem hiding this comment.
it wasn't obvious to me that non-MI shouldn't be allowed through here, this should clarify it for future readers (or me once I forget)
There was a problem hiding this comment.
Gotcha; does that fix an issue somewhere else?
There was a problem hiding this comment.
no, just clarity, avoids doing some other checks before raising
WillAyd
added
Clean
Indexing
pandas/core/frame.py
Outdated
| # NB: we can't just use self.loc[key] = value because that | ||
| # operates on labels and we need to operate positional for | ||
| # backwards-compat, xref GH#31469 | ||
| self._check_setitem_copy() | ||
| self.loc._setitem_with_indexer(key, value) | ||
| self.iloc[key] = value |
There was a problem hiding this comment.
iloc ends up calling the same method, so calling it here directly probably avoids a bunch of duplicate validation.
So I would either leave it as is, or at least check the impact of that, or eg change to iloc._setitem_with_indexer(..) to make it more obvious that it is positional indexing
There was a problem hiding this comment.
WIll do that for now and revisit in a non-"cleanup" PR
| # Validate start & stop | ||
| if start is not None: | ||
| self.get_loc(start) | ||
| if stop is not None: | ||
| self.get_loc(stop) | ||
| is_positional = False | ||
| except KeyError: | ||
| if self.inferred_type in ["mixed-integer-float", "integer-na"]: |
There was a problem hiding this comment.
not reachable; the check for self.is_mixed() a few lines up means these inferred_types are impossible
|
does this change anything user facing? (no?) |
Correct, nothing user-facing. |
|
Thanks! |
