Spenserrrr opened a new pull request, #57693:
URL: https://github.com/apache/spark/pull/57693

   ### What changes were proposed in this pull request?
   
   The column-name varargs methods on `DataFrameWriter` (`partitionBy`, 
`clusterBy`, `bucketBy`, `sortBy`) and `DataStreamWriter` (`partitionBy`, 
`clusterBy`), in both classic and Spark Connect, accept either multiple column 
names as varargs or a single sequence of column names, which they unwrap at 
runtime. Their type annotations did not describe this accurately and relied on 
`# type: ignore` comments. This PR corrects the annotations and removes the 
ignores:
   
   - `partitionBy`/`clusterBy`: the second overload and implementation used 
`List[str]`, but the runtime `isinstance` check already accepted a tuple. 
Widened to `Sequence[str]` (single-sequence overload `__cols: Sequence[str]`, 
implementation `*cols: Union[str, Sequence[str]]`), and the runtime unwrap now 
uses `not isinstance(cols[0], str) and isinstance(cols[0], Sequence)` with 
`cols = tuple(cols[0])`. The streaming implementations were declared `*cols: 
str` with a `# type: ignore[misc]` suppressing the overload mismatch; they are 
now honest and the `[misc]` is removed.
   - `bucketBy`/`sortBy`: replaced the local alias `TupleOrListOfString = 
Union[List[str], Tuple[str, ...]]` with `Sequence[str]` (matching the other 
writer methods), widened the runtime check to match, and normalized the unwrap 
to `col, cols = col[0], tuple(col[1:])`, removing the `# type: 
ignore[assignment]`.
   - `DataFrameNaFunctions.replace`: had the same annotation issue SPARK-56731 
fixed for `DataFrame.replace` (the `Dict` overload allowed `subset` 
positionally after skipping `value`). Made `subset` keyword-only in that 
overload (`*,`) and removed the `# type: ignore[misc]`. Only the `@overload` 
annotations change; the implementation signature is untouched.
   
   This follows the approach in SPARK-55967, which unified and corrected the 
column-conversion annotations for the connect DataFrame (`List` -> `Sequence`).
   
   This is one of a few related PRs cleaning up the "varargs that also accept a 
single sequence" typing pattern across PySpark. A couple of related cases are 
intentionally left for separate discussion: `describe`/`selectExpr` currently 
accept a `list` only (widening them to accept a sequence would be a small 
behavior change), and the column functions 
(`struct`/`create_map`/`array`/`map_concat`) check `(list, set)` in classic vs 
`(list, set, tuple)` in connect, so the honest type there is not simply 
`Sequence`.
   
   ### Why are the changes needed?
   
   The annotations were narrower than the runtime contract (declared `List` 
while the code also accepts a tuple), and the streaming implementations did not 
conform to their `@overload` declarations. Correcting the annotations lets the 
suppression comments be removed and makes the accepted inputs explicit to users 
and type checkers.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No. The annotations and runtime checks are widened to accept any sequence of 
strings (previously `list`, or `list`/`tuple`), which is backward compatible.
   
   ### How was this patch tested?
   
   Existing tests, full-scope `mypy` over `python/pyspark`, and the typing 
tests under `python/pyspark/sql/tests/typing`.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 4.8)


-- 
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]

Reply via email to