REF: share __getitem__ for Categorical/PandasArray/DTA/TDA/PA #36391
Conversation
jreback
added
Indexing
pandas/core/arrays/categorical.py
Outdated
| result = self._codes[key] | ||
| if result.ndim > 1: | ||
| result = super().__getitem__(key) | ||
| if np.ndim(result) > 1: |
There was a problem hiding this comment.
Is np.ndim(result) > 1 generally slower than something like not lib.is_scalar(result) and result.ndim > 1?
result = 5
%timeit np.ndim(result) > 1
2.77 µs ± 48.3 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
%timeit not lib.is_scalar(result) and result.ndim > 1
154 ns ± 1.2 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
or is this not important?
There was a problem hiding this comment.
not lib.is_scalar(result) and result.ndim > 1?
The trouble here is if we have a tuple result. I'll take a look at finding a more performant solution.
There was a problem hiding this comment.
I did wonder about that. but pandas\tests\arrays\categorical and pandas\tests\extension\test_categorical.py didn't fail with this change. Do we have tests for this.
also is it possible to get a nested tuple result? np.ndim would give 2 in this case?
There was a problem hiding this comment.
Looks like getattr(result, "ndim", 1) > 1 outperforms both np.ndim and lib.is_scalar checks, and dont need to worry about tuples. Will update.
also is it possible to get a nested tuple result? np.ndim would give 2 in this case?
Yes. Supporting list-like object-dtype is a massive PITA.
| result = self._from_backing_data(result) | ||
| return result | ||
|
|
||
| def _validate_getitem_key(self, key): |
There was a problem hiding this comment.
this duplicates _validate_setitem_key. is this needed?
in what cases could one of a getitem and a setitem key be valid and other invalid?
There was a problem hiding this comment.
DatetimeLikeArrayMixin has special logic here. #36210 is about deprecating that
|
@jbrockmendel merge away if good here (likely address @simonjayhawkins points as followups?) |
| @@ -30,6 +31,9 @@ def _from_backing_data(self: _T, arr: np.ndarray) -> _T: | |||
| """ | |||
| raise AbstractMethodError(self) | |||
|
|
|||
| def _box_func(self, x): | |||
There was a problem hiding this comment.
Can you add a docstring for this function?
No description provided.