CLN/REF: indexing typing, prune unreachable branches #27351
Conversation
| @@ -2889,11 +2889,11 @@ def _set_value(self, index, col, value, takeable=False): | |||
|
|
|||
| _set_value.__doc__ = set_value.__doc__ | |||
|
|
|||
| def _ixs(self, i, axis=0): | |||
| def _ixs(self, i: int, axis: int = 0): | |||
There was a problem hiding this comment.
Since axis is a pretty common keyword can you put that as Axis in pandas._typing? We probably also want to add str as a type for it since it should accept "index" and "columns" as well
There was a problem hiding this comment.
Also can we add the return type here? -> 'DataFrame'?
There was a problem hiding this comment.
I'll take a look and see what we can determine about return types. For Axis/str/int, in all these cases, I've put int because these are private methods and only ever get ints.
| if axis is None: | ||
| axis = self.axis or 0 | ||
|
|
||
| def _get_label(self, label, axis: int): |
| @@ -656,6 +578,65 @@ def setter(item, v): | |||
| self.obj._data = self.obj._data.setitem(indexer=indexer, value=value) | |||
| self.obj._maybe_update_cacher(clear=True) | |||
|
|
|||
| def _setitem_with_indexer_missing(self, indexer, value): | |||
There was a problem hiding this comment.
itd just be Any for both of em. Is that materially better than leaving it blank? In general I prefer to leave it blank rather than put a too-loose type that I'm not sure about
| def _slice(self, obj, axis=None, kind=None): | ||
| if axis is None: | ||
| axis = self.axis | ||
| def _slice(self, obj, axis: int, kind=None): |
| @@ -656,6 +578,65 @@ def setter(item, v): | |||
| self.obj._data = self.obj._data.setitem(indexer=indexer, value=value) | |||
| self.obj._maybe_update_cacher(clear=True) | |||
|
|
|||
| def _setitem_with_indexer_missing(self, indexer, value): | |||
| """ | |||
There was a problem hiding this comment.
can you add a doc-string though
There was a problem hiding this comment.
I'd rather not. If I did, it would just say both parameters are object, which may be too loose. I'd rather leave it open for someone to reason about and add a more-correct version.
There was a problem hiding this comment.
actually I think spitting this into a ndim==1 and a ndim==2 functions would be helpful (future PR ok)
|
still would like to move to a unified pandas/core/indexing/..... structure sooner rather than later |
| @@ -3495,7 +3495,7 @@ def __delitem__(self, key): | |||
| deleted = False | |||
|
|
|||
| maybe_shortcut = False | |||
| if hasattr(self, "columns") and isinstance(self.columns, MultiIndex): | |||
| if self.ndim == 2 and isinstance(self.columns, MultiIndex): | |||
There was a problem hiding this comment.
in the future change to use ABCMultiIndex
|
ok to merge, though if you want to split the setting routine I think might be helpful here. |
will do in next round. lots of balls (branches) in the air |
k |
|
thanks |
No description provided.