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

   ### What changes were proposed in this pull request?
   
   Removes 16 `# type: ignore[union-attr]` comments across 4 files in 
`python/pyspark` by making the implicit non-None assumption explicit at each 
access site:
   
   - **`sql/types.py` (9):** In `_parse_datatype_json_value`, each atomic-type 
branch matched its regex twice - once in the `elif` test and again to bind `m` 
- then suppressed the `Optional[re.Match]` on `m.group(...)`. Converting these 
to the walrus form (`elif m := PATTERN.match(json_value):`) matches once, 
narrows `m` for the branch body, and drops the ignore. This also removes the 
redundant second match and makes these branches consistent with the 
geometry/geography branches in the same function, which already use 
single-match-plus-guard.
   - **`errors/exceptions/captured.py` (5):** The `CapturedException` accessors 
access `SparkContext._jvm.PythonErrorUtils`, where `_jvm` is 
`Optional[JVMView]`. Each method already asserts `SparkContext._gateway is not 
None`; adding the matching `assert SparkContext._jvm is not None` (the same 
idiom already used elsewhere in the file) narrows `_jvm` and removes the ignore.
   - **`sql/connect/client/core.py` (1):** The code narrowed a local `session` 
via `if session is not None:` but then re-read the Optional class attribute 
`PySparkSession._instantiatedSession._jvm`. Using the narrowed local 
(`session._jvm`) removes the ignore and the redundant re-read.
   - **`sql/catalog.py` (1):** The code asserted `sc is not None` but the 
ignore was about `sc._gateway` being Optional. Adding `assert sc._gateway is 
not None` completes the guard the code had already started.
   
   ### Why are the changes needed?
   
   These ignores suppressed mypy `[union-attr]` errors that arise only because 
a value is typed Optional at the access site, even though the surrounding code 
guarantees it is not None. Making the assumption explicit is clearer and 
matches idioms the codebase already uses nearby. The `assert` additions do not 
introduce a new failure mode - the affected code already required a live 
JVM/gateway on these paths.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   `mypy` passes at full scope over `python/pyspark` (1287 source files). No 
behavior change; existing tests cover the affected code paths.
   
   ### 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