REVIEW: [BUG] Make pyspark an optional extra on overture-schema-pyspark

.pr-review/BODY.md

@@ -0,0 +1,47 @@
+Closes #659. Supersedes #660, which is where this change and its changelog wording originated; @Rachmanin0xFF is credited as co-author on the commit.
+
+## Problem
+
+`overture-schema-pyspark` declared `pyspark>=3.4` as a hard dependency, so installing the package resolved `pyspark` on every runtime — including ones (Glue, EMR) that already bundle their own PySpark and shouldn't have this package re-resolving a different version underneath them.
+
+## Change
+
+`pyspark` moves to an optional extra:
+
+```toml
+[project.optional-dependencies]
+spark = ["pyspark>=3.4"]
+```
+
+`pip install overture-schema-pyspark` now resolves cleanly against a runtime-provided PySpark. Standalone environments building their own install `overture-schema-pyspark[spark]`.
+
+That makes "PySpark isn't here" an expected first-run state rather than a broken install, so the package says what to do about it. `overture/schema/pyspark/__init__.py` probes for pyspark before its own imports:
+
+```python
+if importlib.util.find_spec("pyspark") is None:
+    raise ModuleNotFoundError(
+        "overture-schema-pyspark requires PySpark, which isn't installed. "
+        "Install it with `pip install overture-schema-pyspark[spark]`, or run "
+        "in an environment that already provides PySpark (e.g. a Spark cluster)."
+    )
+```
+
+Two properties are worth stating, because they're what make one probe sufficient:
+
+**It covers every entry path.** `overture/schema/pyspark/__init__.py` is a real `__init__.py` — only `overture` and `overture.schema` are PEP 420 namespaces — so it executes on any import of any submodule, including the 15 generated expression modules that import `pyspark.sql` directly. Guarding at those import sites instead would mean 22 guards (7 hand-written modules plus the generated tree, which would need a renderer change), of which only one is ever reachable: the re-export chain always hits `check.py` first.
+
+**It catches nothing.** A `try`/`except ModuleNotFoundError` around the imports would also swallow an unrelated missing dependency, and — less obviously — would misreport an installed-but-broken pyspark. With `py4j` removed, `import pyspark` raises `ModuleNotFoundError(name='py4j')`; an `except` clause reports "PySpark isn't installed", which is false, and avoiding that requires inspecting `e.name`. The probe has no such branch: it asks whether pyspark is findable, and everything else surfaces its own real error from the imports that follow.
+
+## Tests
+
+`tests/test_optional_pyspark.py` adds three tests. One is the gate: `test_missing_pyspark_names_the_extra` fails against the unguarded `__init__` and passes with the probe. The other two are guards, and pass both before and after — `test_unrelated_missing_dependency_surfaces_its_own_error` pins the no-masking property (verified to fail against a broad-`except` implementation), and `test_package_imports_when_pyspark_is_installed` pins the normal path.
+
+The two blocking mechanisms differ on purpose. The pyspark test sets `sys.modules["pyspark"] = None`, which makes `find_spec` return `None` — the same signal real absence produces at the seam this code reads. The unrelated-dependency test uses a `meta_path` finder that raises `ModuleNotFoundError(..., name=...)`, because `sys.modules[x] = None` raises `ImportError` instead, and a control built on it would pass under the very implementation it exists to forbid.
+
+## Verified
+
+- `make check TESTMON=` — 6201 passed.
+- Bare install in a clean venv (`uv pip install ./packages/overture-schema-system ./packages/overture-schema-pyspark`, no pyspark): importing the package raises the actionable message; the automated test pins the branch, this pins that `find_spec` is the right question.
+- Same venv with `shapely` also removed, and the worktree venv with `py4j` removed: both surface their own errors, unmasked.
+- `uv sync --locked --all-packages --all-extras` accepts the lockfile; CI's `make check` installs all extras, so coverage is unaffected.
+- No other package in this repo depends on `overture-schema-pyspark` or imports `pyspark`.

packages/overture-schema-pyspark/changelog.d/659.bugfix.md

@@ -0,0 +1 @@
+Moved `pyspark` from a hard dependency to the `spark` optional extra, so installing `overture-schema-pyspark` no longer resolves `pyspark` on runtimes (Glue, EMR) that already bundle their own. Standalone environments need `pip install overture-schema-pyspark[spark]`. Importing the package without PySpark installed now raises an actionable `ModuleNotFoundError` pointing at that extra, instead of a bare "No module named 'pyspark'".

packages/overture-schema-pyspark/pyproject.toml

@@ -6,14 +6,16 @@ build-backend = "uv_build"
 dependencies = [
     "click>=8.0",
     "overture-schema-system>=0.1.1",
-    "pyspark>=3.4",
 ]
 description = "PySpark validation expressions for Overture Maps data"
 license = "MIT"
 name = "overture-schema-pyspark"
 requires-python = ">=3.10"
 version = "0.1.1"
 
+[project.optional-dependencies]
+spark = ["pyspark>=3.4"]
+
 [project.scripts]
 overture-validate = "overture.schema.pyspark.cli:validate_cli"
 

packages/overture-schema-pyspark/src/overture/schema/pyspark/__init__.py

@@ -1,5 +1,20 @@
 """PySpark validation expressions for Overture Maps data."""
 
+import importlib.util
+
+# pyspark is an optional extra (the `spark` extra): a bare install lets this
+# package's metadata and console script resolve, but every module below needs
+# pyspark itself to do anything. Probe for it here -- this module runs on any
+# import of any submodule -- so a bare install gets an actionable message. A
+# pyspark that is present but broken still raises its own error, because the
+# imports below run for real.
+if importlib.util.find_spec("pyspark") is None:
+    raise ModuleNotFoundError(
+        "overture-schema-pyspark requires PySpark, which isn't installed. "
+        "Install it with `pip install overture-schema-pyspark[spark]`, or run "
+        "in an environment that already provides PySpark (e.g. a Spark cluster)."
+    )
+
 from .check import Check, CheckShape
 from .schema_check import SchemaMismatch, compare_schemas
 from .validate import (

packages/overture-schema-pyspark/tests/test_optional_pyspark.py

@@ -0,0 +1,75 @@
+"""Tests for the missing-PySpark guard in the package's __init__."""
+
+import importlib
+import sys
+from types import ModuleType
+
+import pytest
+
+
+class _BlockedFinder:
+    """A meta_path finder that reports one module as genuinely absent.
+
+    Raising from `find_spec` reproduces what an uninstalled module does at an
+    `import` statement -- a `ModuleNotFoundError` carrying `name`. Setting
+    `sys.modules[name] = None` would raise `ImportError` instead, which is not
+    the error a missing package actually produces.
+    """
+
+    def __init__(self, name: str) -> None:
+        self.name = name
+
+    def find_spec(
+        self, fullname: str, path: object = None, target: ModuleType | None = None
+    ) -> None:
+        if fullname == self.name or fullname.startswith(f"{self.name}."):
+            raise ModuleNotFoundError(f"No module named {fullname!r}", name=fullname)
+        return None
+
+
+def _unload(monkeypatch: pytest.MonkeyPatch, *roots: str) -> None:
+    """Drop modules from sys.modules so importing them re-runs their top level.
+
+    monkeypatch restores the original module objects at teardown, so modules
+    already imported by the rest of the suite keep their identity.
+    """
+    for name in [
+        name
+        for name in sys.modules
+        if any(name == root or name.startswith(f"{root}.") for root in roots)
+    ]:
+        monkeypatch.delitem(sys.modules, name)
+
+
+def test_missing_pyspark_names_the_extra(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setitem(sys.modules, "pyspark", None)
+    _unload(monkeypatch, "overture.schema.pyspark")
+
+    with pytest.raises(ModuleNotFoundError) as excinfo:
+        importlib.import_module("overture.schema.pyspark")
+
+    assert "overture-schema-pyspark[spark]" in str(excinfo.value)
+
+
+def test_unrelated_missing_dependency_surfaces_its_own_error(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """The guard reports only a missing pyspark, never another package's."""
+    _unload(monkeypatch, "overture.schema.pyspark", "overture.schema.system", "shapely")
+    monkeypatch.setattr(sys, "meta_path", [_BlockedFinder("shapely"), *sys.meta_path])
+
+    with pytest.raises(ImportError) as excinfo:
+        importlib.import_module("overture.schema.pyspark")
+
+    assert "shapely" in str(excinfo.value)
+    assert "overture-schema-pyspark[spark]" not in str(excinfo.value)
+
+
+def test_package_imports_when_pyspark_is_installed(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    _unload(monkeypatch, "overture.schema.pyspark")
+
+    module = importlib.import_module("overture.schema.pyspark")
+
+    assert module.Check.__name__ == "Check"