Skip to content

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

Open
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:nonetype-guards
Open

[SPARK-58346][PYTHON] Remove unnecessary type: ignore[union-attr] comments in PySpark#57521
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:nonetype-guards

Conversation

@Spenserrrr

@Spenserrrr Spenserrrr commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

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 pattern 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)

…ments in PySpark

### 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): convert the atomic-type branches in
  `_parse_datatype_json_value` to the walrus form
  (`elif m := PATTERN.match(json_value):`), removing both the ignore and a
  redundant second regex match.
- `errors/exceptions/captured.py` (5): add `assert SparkContext._jvm is not
  None` alongside the existing `assert SparkContext._gateway is not None`.
- `sql/connect/client/core.py` (1): use the already-narrowed local `session`
  instead of re-reading `PySparkSession._instantiatedSession._jvm`.
- `sql/catalog.py` (1): add `assert sc._gateway is not None`, completing the
  guard the code had already started with `assert sc is not None`.

### 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.

### 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)
@Spenserrrr
Spenserrrr marked this pull request as ready for review July 25, 2026 01:25
@Spenserrrr

Copy link
Copy Markdown
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant