You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Spenserrrr (via GitHub)" <gi...@apache.org> on 2026/07/25 01:13:29 UTC

[PR] [SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark [spark]

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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark [spark]

Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #57521:
URL: https://github.com/apache/spark/pull/57521#issuecomment-5083085822

   Nice, thank you @Spenserrrr!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark [spark]

Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian commented on PR #57521:
URL: https://github.com/apache/spark/pull/57521#issuecomment-5095424735

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/149ba07b5606a1f9443b313d74bbee8baa717ff4
   - merged into branch-4.x https://github.com/apache/spark/commit/7e1a997a1079fd6394f2836e44de7bd330205013
   
   *Posted by `merge_spark_pr.py`*


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark [spark]

Posted by "gaogaotiantian (via GitHub)" <gi...@apache.org>.
gaogaotiantian closed pull request #57521: [SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark
URL: https://github.com/apache/spark/pull/57521


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark [spark]

Posted by "Spenserrrr (via GitHub)" <gi...@apache.org>.
Spenserrrr commented on PR #57521:
URL: https://github.com/apache/spark/pull/57521#issuecomment-5076062646

   Hi @gaogaotiantian! This is a PR on type: ignore. Please let me know if there is anything I need to change.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org