Assorted cleanups #27376
Assorted cleanups #27376
Conversation
| new_values = algos.take_nd( | ||
| values, indexer, axis=axis, allow_fill=True, fill_value=fill_value | ||
| ) | ||
| allow_fill = True |
There was a problem hiding this comment.
In the ExtensionBlock.take_nd method we have this same if/elif for fill_tuple/fill_value, but we don't set allow_fill there. Seems fishy. @jorisvandenbossche any idea if this is intentional?
There was a problem hiding this comment.
Do you know for what case the allow_fill=False branch gets hit?
In the end, I think this is only an optimization for when you know that there are no -1's in the indexer. Because in the internals, I think we never use the numpy-like indexing semantics of -1 meaning the last element.
pandas/core/indexing.py
Outdated
| @@ -1408,7 +1408,7 @@ def __getitem__(self, key): | |||
| maybe_callable = com.apply_if_callable(key, self.obj) | |||
| return self._getitem_axis(maybe_callable, axis=axis) | |||
|
|
|||
| def _is_scalar_access(self, key): | |||
| def _is_scalar_access(self, key: tuple): | |||
There was a problem hiding this comment.
Can you use Tuple from typing module and annotate types contained within if possible?
|
|
||
| # Called from three places in managers, all of which satisfy | ||
| # this assertion | ||
| assert not (axis == 0 and new_mgr_locs is None) |
There was a problem hiding this comment.
Can you raise a ValueError here instead of using assert? In the off chance someone uses -O flag assert won't help
There was a problem hiding this comment.
asserts are ok in the internals
|
tuple change made |
| @@ -1408,7 +1409,7 @@ def __getitem__(self, key): | |||
| maybe_callable = com.apply_if_callable(key, self.obj) | |||
| return self._getitem_axis(maybe_callable, axis=axis) | |||
|
|
|||
| def _is_scalar_access(self, key): | |||
| def _is_scalar_access(self, key: Tuple): | |||
There was a problem hiding this comment.
Length here is always indeterminate right? If it is predefined would be nice to subscript with the length of elements
There was a problem hiding this comment.
Yah, in general we don't know the length.
I'll be doing some more work in this file, will try to make types more specific as I figure them out
|
anything else needed here? |
|
thanks! |
The only noticeable thing should be a couple of fixed build warnings from khash_python