Spenserrrr commented on code in PR #58021:
URL: https://github.com/apache/spark/pull/58021#discussion_r3811540509
##########
python/pyspark/pandas/base.py:
##########
@@ -542,12 +542,14 @@ def __len__(self) -> int:
# NDArray Compat
def __array_ufunc__(
self, ufunc: Callable, method: str, *inputs: Any, **kwargs: Any
- ) -> SeriesOrIndex:
+ ) -> Union[SeriesOrIndex, Tuple[SeriesOrIndex, ...]]:
Review Comment:
Thanks @Yicong-Huang! I changed the annotation as you suggested, and cleaned
up a couple of other type problems along the way in the two new commits. On the
two points you mentioned:
1. Done: The two `maybe_dispatch_ufunc_to_*` helpers were typed to return
the base `IndexOpsMixin` even though they already produce `SeriesOrIndex` (via
column_op), which is what forced the cast(...) in `__array_ufunc__`. Both now
return `SeriesOrIndex`, matching column_op and the data_type_ops operators, so
the cast is gone. I also narrowed the multi-output tuple to exactly two
elements per your suggestion.
2. For the second point on base class referring to its subclasses, that
annotation is accurate as the methods really do return a Series or an Index.
The more correct way would indeed be for the base to return IndexOpsMixin
itself instead of its subclasses. However, the downside is that callers then
receive the base type and have to cast(...) it back to a Series/Index to use
it, which is the same problem we remove on the unfunc helper. Also, this
problem is not confined to IndexOpsMixin: the data_type_ops/ operators (a
separate class hierarchy) return SeriesOrIndex throughout too, so the same
change would ripple across that whole layer.
Hi @gaogaotiantian, since this is a typing design question, could you take a
look at the second point as well, if you have time? Thank you so much!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
