TYP: __getitem__ method of EA (2nd pass) #37921
Conversation
simonjayhawkins
added
ExtensionArray
pandas/core/internals/blocks.py
Outdated
| @@ -484,6 +484,7 @@ def split_and_operate( | |||
| ------- | |||
| list of blocks | |||
| """ | |||
| assert isinstance(self.values, np.ndarray) | |||
There was a problem hiding this comment.
need to ensure split_and_operate is not called from EA block (or base method not overridden)
There was a problem hiding this comment.
move assert after ndim check
| # TODO: overload DatetimeLikeArrayMixin.__getitem__ | ||
| values = cast(PeriodArray, self._data[:0]) | ||
| return type(self)._simple_new(values, name=self.name) | ||
| return type(self)._simple_new(self._data[:0], name=self.name) |
There was a problem hiding this comment.
reverting change from previous PR
| first_value = cast(Any, self._values[mask.argmax()]) | ||
| result[(locs == 0) & (where._values < first_value)] = -1 | ||
| first = mask.argmax() | ||
| result[(locs == 0) & (where._values < self._values[first])] = -1 |
There was a problem hiding this comment.
reverting change from previous PR
|
lgtm. @jbrockmendel |
| @overload | ||
| # error: Overloaded function signatures 1 and 2 overlap with incompatible | ||
| # return types [misc] | ||
| def __getitem__(self, key: int) -> EAScalarOrMissing: # type: ignore[misc] |
There was a problem hiding this comment.
this can also return NDArrayBackedExtensionArrayT
There was a problem hiding this comment.
can you elaborate. is this for 2d EA?
There was a problem hiding this comment.
NDArrayBackedExtensionArray supports 2D, exactly
There was a problem hiding this comment.
so it also supports a tuple indexer? and I guess from that NDArrayBackedExtensionArray can't support nested data as it uses is_scalar checks.
so also need to change EAScalarOrMissing = object # both scalar value and na_value can be any type -> ScalarOrScalarMissing = Scalar # both values and na_value must be scalars ?
There was a problem hiding this comment.
so it also supports a tuple indexer?
Yes. For that matter i think even 1D will support 1-tuples
and I guess from that NDArrayBackedExtensionArray can't support nested data as it uses is_scalar checks.
PandasArray can have object dtype, and Categorical can hold tuples. We never see 2D versions of those in practice though (yet).
| @@ -266,9 +267,19 @@ def __array__(self, dtype=None) -> np.ndarray: | |||
| return np.array(list(self), dtype=object) | |||
| return self._ndarray | |||
|
|
|||
| @overload | |||
| def __getitem__(self, key: int) -> DTScalarOrNaT: | |||
There was a problem hiding this comment.
same, can still return DatetimeLikeArrayT
|
couple of comments, otherwise LGTM |
|
removing this from the 1.2 milestone for now. since #35259 is merged, can now proceed with the outstanding tasks in parallel. so plan is to get all the tasks started (to get review) and will then circle back to this. |
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
|
closing while we discuss #39513 |
follow-on from #37898
remove casts and ignores added in #37898 (needed to use typevar in DatetimeLikeArrayMixin in conjunction with overload for slice to return correct DatetimeArray/PeriodArray)
another pass required to
more typevars for return type of type(self)
type __getitem__ in concrete classes (Categorical, IntervalArray and SparseArray)
overloads with Any report overlapping overloads
NAType resolves to
Anysince implemented in cython without stub file