HyukjinKwon commented on code in PR #56284:
URL: https://github.com/apache/spark/pull/56284#discussion_r3449197258
##########
python/pyspark/pandas/frame.py:
##########
@@ -3596,9 +3597,8 @@ def xs(self, key: Name, axis: Axis = 0, level:
Optional[int] = None) -> DataFram
----------
key : label or tuple of label
Label contained in the index, or partially in a MultiIndex.
- axis : 0 or 'index', default 0
+ axis : {0 or 'index', 1 or 'columns'}, default 0
Review Comment:
Nit: consider adding an `axis=1` doctest `Example` to this docstring. The
unit tests cover the new behavior, but pandas-on-Spark docstrings are
doctested, so an example here would both document the new capability and
exercise it in CI.
##########
python/pyspark/pandas/frame.py:
##########
@@ -3681,11 +3758,13 @@ class locomotion
len(key), self._internal.index_level
)
)
- if level is None:
- level = 0
+ index_level = cast(Optional[int], level)
Review Comment:
`level` is widened to `Optional[Union[int, Name]]` so the new `axis=1` path
can resolve a level name, but the index path keeps `index_level =
cast(Optional[int], level)` and then `enumerate(key, index_level)`. So `xs(key,
axis=0, level=<name>)` raises a cryptic `TypeError: 'str' object cannot be
interpreted as an integer`, whereas pandas supports index level-by-name (e.g.
`df.xs('mammal', level='class')`).
Non-blocking and pre-existing at runtime, but the widened type now
advertises support the index axis doesn't actually provide. Consider either
resolving names on the index axis too, or guarding with a clear error when a
name level is passed with `axis=0`.
--
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]
