EdwardLeeMacau opened a new pull request, #59379:
URL: https://github.com/apache/spark/pull/59379
### What changes were proposed in this pull request?
Removes 5 `# type: ignore[union-attr]` comments in `python/pyspark` that
suppress mypy errors caused by `SparkContext._jvm` and `SparkContext._gateway`
being typed `Optional` (they are `None` until a `SparkContext` is created), by
making the non-None assumption explicit at each access site:
- **`sql/connect/session.py` (1)**, **`sql/streaming/listener.py` (2)**: add
`assert SparkContext._jvm is not None` before the access. In
`StreamingQueryProgress.fromJObject` the assert goes before the `return
cls(...)`, since the access is inside a dict comprehension.
- **`ml/connect/io_utils.py` (1)**: read the gateway into a local variable
and assert it is not None.
- **`ml/wrapper.py` (1)**: in `JavaWrapper.__del__`, read
`SparkContext._active_spark_context` into a local variable and add `sc._gateway
is not None` to the existing condition. This method already catches and ignores
any `Exception`, so an assert would add nothing here.
### Why are the changes needed?
The ignores hide an assumption the code already relies on: these paths need
a live JVM. Stating it with an `assert` documents the invariant in a form the
type checker understands. It matches the idiom `SparkContext` itself uses (for
example `assert self._jvm is not None` in `core/context.py`) and SPARK-58346.
### Does this PR introduce _any_ user-facing change?
No. At the four sites that now use `assert`, a missing JVM handle raises
`AssertionError` instead of `AttributeError`. One of them,
`StreamingQueryProgress.fromJObject`, now checks eagerly instead of only when
an event has observed metrics; it is only called from JVM callbacks, so a live
JVM is guaranteed. `JavaWrapper.__del__` behaves the same as before.
### How was this patch tested?
- `mypy --namespace-packages --config-file python/mypy.ini python/pyspark`
passes; `dev/lint-python --compile --custom-pyspark-error --ruff` reports no
problems.
- Existing tests pass locally:
- `pyspark.sql.tests.streaming.test_streaming_listener`: `addListener`
reaches `_jlistener`, and JVM callbacks reach
`StreamingQueryProgress.fromJObject`, including the observed-metrics path.
- `pyspark.ml.tests.test_wrapper`: calls `JavaWrapper.__del__` directly
and checks the Java object is detached.
- `pyspark.ml.tests.connect.test_legacy_mode_classification`:
`estimator.save(...)` and `model.save(...)` reach the non-remote branch of
`_copy_file_from_local_to_fs`.
- Not run locally: the local-server startup path in
`sql/connect/session.py`, which is covered by Connect tests that start a local
server; I relied on CI for it.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code 2.1.296 (claude-sonnet-5-5)
I confirm that this contribution is my original work, with the generative AI
assistance disclosed above, and that I license it to the project under the
Apache License 2.0 (the project's open source license).
--
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]
