[
https://issues.apache.org/jira/browse/SPARK-58346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Haotian Sun updated SPARK-58346:
--------------------------------
Description:
Several `# type: ignore[union-attr]` comments in `python/pyspark` suppress mypy
errors that arise only because a value is typed as Optional (or a union
containing None) at the access site, even though the surrounding code
guarantees it is not None. These ignores can be removed by making the non-None
assumption explicit, which is both clearer and closer to patterns the codebase
already uses nearby.
This change removes 16 such ignores across 4 files, using the appropriate
narrowing idiom in each case:
- `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. The assert does not introduce a new failure mode - the code already
required a live JVM here.
- `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.
No behavior change. `mypy` passes at full scope over `python/pyspark`.
was:
Several `# type: ignore[union-attr]` comments in `python/pyspark` suppress
mypy errors that arise only because a value is typed as Optional (or a union
containing None) at the access site, even though the surrounding code
guarantees it is not None. These ignores can be removed by making the
non-None assumption explicit, which is both clearer and closer to patterns
the codebase already uses nearby.
This change removes 16 such ignores across 4 files, using the appropriate
narrowing idiom in each case:
- `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. The assert does
not introduce a new failure mode - the code already required a live JVM here.
- `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.
No behavior change. `mypy` passes at full scope over `python/pyspark`.
> Remove unnecessary type: ignore[union-attr] comments in PySpark
> ---------------------------------------------------------------
>
> Key: SPARK-58346
> URL: https://issues.apache.org/jira/browse/SPARK-58346
> Project: Spark
> Issue Type: Improvement
> Components: PySpark
> Affects Versions: 4.3.0
> Reporter: Haotian Sun
> Priority: Minor
>
> Several `# type: ignore[union-attr]` comments in `python/pyspark` suppress
> mypy errors that arise only because a value is typed as Optional (or a union
> containing None) at the access site, even though the surrounding code
> guarantees it is not None. These ignores can be removed by making the
> non-None assumption explicit, which is both clearer and closer to patterns
> the codebase already uses nearby.
> This change removes 16 such ignores across 4 files, using the appropriate
> narrowing idiom in each case:
> - `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. The assert does not introduce a new failure mode - the code already
> required a live JVM here.
> - `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.
> No behavior change. `mypy` passes at full scope over `python/pyspark`.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
