geotiff: GPU writer + dispatcher coverage for CRS fail-closed (#1929)

[brendancol]

Summary

Closes a Cat 1 HIGH backend-parity gap for issue #1929 (CRS fail-closed). test_crs_fail_closed_1929.py only exercises the eager CPU writer; the GPU writer's call to _validate_crs_fallback at _writers/gpu.py:507 and the to_geotiff(gpu=True) dispatcher thread-through at _writers/eager.py:447 had no targeted tests.

A regression dropping either call would let write_geotiff_gpu(crs=\"EPSG:4326\") on a host without pyproj, or any other unparseable CRS string, silently emit a garbage GTCitationGeoKey that non-libgeotiff readers cannot interpret.

13 new tests, all passing on a CUDA host:

• write_geotiff_gpu(crs=garbage) raises ValueError (via kwarg + attr)
• allow_unparseable_crs=True opt-in restores citation-only write
• Error message points to all four recovery options (EPSG, WKT, pyproj, opt-in)
• EPSG int + WKT-shaped + no-CRS happy paths still work
• to_geotiff(gpu=True) auto-dispatches on cupy data and threads through
• to_geotiff(gpu=True, allow_unparseable_crs=True) forwards opt-in
• GPU vs CPU error message parity
• Structural pin: allow_unparseable_crs default is False on both writers

Mutation against the GPU validator at _writers/gpu.py:507 flipped 6 tests red. Mutation against the dispatcher forward at _writers/eager.py:447 flipped the two opt-in dispatcher tests red.

This is a deep-sweep test-coverage PR. No source files were modified.

Test plan

• pytest xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py (13 passed)
• pytest xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py xrspatial/geotiff/tests/test_crs_fail_closed_1929.py (47 passed)
• Mutation: drop validator at _writers/gpu.py:507 -> 6 tests fail
• Mutation: drop allow_unparseable_crs forward at _writers/eager.py:447 -> 2 opt-in tests fail

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[brendancol]

PR Review: geotiff: GPU writer + dispatcher coverage for CRS fail-closed (#1929)

Blockers (must fix before merge)

• None

Suggestions (should fix, not blocking)

• xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py:240 — TestKwargDefaultParity.test_default_is_false_on_all_writers iterates only over to_geotiff and write_geotiff_gpu, but _write_vrt_tiled in _writers/eager.py:766 also carries the allow_unparseable_crs kwarg and routes through the same fallback at _writers/eager.py:858. Extending the parity loop to import and check _write_vrt_tiled would pin the default across every writer that actually calls _validate_crs_fallback.
• xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py:147,180,196 — the dispatcher tests assert ValueError and os.path.exists, but never confirm the dispatcher actually reached the GPU writer (vs. silently falling back to CPU on ImportError per _writers/eager.py:450). A small monkeypatch on write_geotiff_gpu to record a call, or an assertion on a GPU-only side effect, would catch a regression that re-routes to CPU and still passes today.
• xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py:227 — TestErrorMessageParity.test_gpu_vs_cpu_message_matches uses == on the full message. The eager and GPU writers compose their own ValueError strings independently; if either ever interpolates a backend-specific hint (e.g. "GPU writer:" prefix) this test breaks even when both paths are correct. A set(...).issubset(...) over the recovery keywords ("EPSG", "WKT", "pyproj", "allow_unparseable_crs") is more durable.

Nits (optional improvements)

• xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py:71,82,93 — pytest.warns(Warning) is very loose. Tightening to the actual warning category (likely UserWarning from _validate_crs_fallback) documents intent and catches a category drift.
• xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py:30 — _gpu_available swallows every Exception. Narrowing to (ImportError, RuntimeError) would surface a real cupy bug instead of skipping the file silently.
• xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py:121 — the WKT used here is missing AXIS and AUTHORITY clauses real readers expect. Fine for _looks_like_wkt which only checks the root keyword, but worth a comment so a future reader does not copy it elsewhere.

What looks good

• Test file is purely additive, no source changes. Imports are deferred inside each test so the module imports cleanly on a CPU-only host before pytestmark skips.
• Mutation evidence in the PR body (drop validator -> 6 fail; drop forward -> 2 fail) is concrete and points at the right lines (_writers/gpu.py:507, _writers/eager.py:447).
• Coverage hits the kwarg path, the attr path, the opt-in escape hatch, the EPSG/WKT/no-CRS happy paths, and the dispatcher for both cupy-backed and gpu=True numpy-backed inputs.
• Unique tmp filenames per test (matches the project tmp-collision convention).

Checklist

• Algorithm matches reference/paper (test-only PR, no algorithm changes)
• All implemented backends produce consistent results (parity test covers CPU vs GPU error)
• NaN handling is correct (not applicable, CRS path)
• Edge cases are covered by tests (kwarg/attr, opt-in, no-CRS, EPSG int, WKT-shaped)
• Dask chunk boundaries handled correctly (not applicable, eager + GPU writers only)
• No premature materialization or unnecessary copies (test-only)
• Benchmark exists or is not needed (not needed)
• README feature matrix updated (if applicable) — not applicable, no user-facing surface change
• Docstrings present and accurate (each test class has a clear docstring)

[brendancol]

PR Review: geotiff: GPU writer + dispatcher coverage for CRS fail-closed (#1929) (re-review after rebase)

Scope after rebase onto 9ce0e60: one new test module, xrspatial/geotiff/tests/test_crs_fail_closed_gpu_1929.py (330 lines). No production code touched.

Blockers

• None

Suggestions

• test_crs_fail_closed_gpu_1929.py:75-78: pytest.warns(Warning) wrapping the pytest.raises block is broad. Warning matches anything, including a future unrelated DeprecationWarning from xarray or cupy. Consider pinning to the specific warning class (e.g. the writer's own GeoTIFFCitationOnlyWarning or whichever class _validate_crs_fallback emits). Applies to every pytest.warns(Warning) block in the file. Not blocking, but tightens the contract.
• test_crs_fail_closed_gpu_1929.py:166-179: the spy installer relies on xrspatial.geotiff._writers.eager.write_geotiff_gpu being the dispatcher's call target. That is the case today (line 433 of eager.py), but a future refactor that imports write_geotiff_gpu inside the dispatcher function (rather than at module top) would silently bypass the monkeypatch and the assertion would still pass via the real GPU call. A brief comment in _install_gpu_spy noting the dispatch site (_writers/eager.py:433) it pins would make the coupling explicit.

Nits

• test_crs_fail_closed_gpu_1929.py:131-135: the WKT string uses mixed single- and double-quote escaping inside the same multi-line literal. Functionally fine, just a touch hard to scan. A single raw triple-quoted string would read cleaner.
• test_crs_fail_closed_gpu_1929.py:53: np.arange(4.0, 0, -1) evaluates to [4., 3., 2., 1.]; np.arange(1.0, 5.0)[::-1] or an explicit array would read more obviously.

What looks good

• The "spy on _eager.write_geotiff_gpu and assert calls is non-empty" pattern is the right defense against the ImportError fallback at _writers/eager.py:451 silently turning a GPU test green via the CPU path. The docstring at lines 156-164 explains exactly why, which will help future readers.
• TestKwargDefaultParity introspects the signatures of all three writers (to_geotiff, write_geotiff_gpu, _write_vrt_tiled) and pins each default to False. That is the smallest possible regression net for the geotiff: CRS resolution should fail closed on unparseable strings by default #1929 fail-closed contract and is independent of any runtime behaviour.
• TestErrorMessageParity asserts substring presence rather than full-string equality, which is the right tradeoff for cross-backend message drift.
• GPU skip gate at lines 28-42 correctly checks both cupy import and cupy.cuda.is_available() before declaring the suite available, so a CPU-only CI box still collects cleanly.
• The 13 tests pass locally after the rebase: 13 passed in 1.20s.

Checklist

• Rebase onto origin/main clean (rebased over 9ce0e60).
• Target test file passes: 13/13 green.
• No production code modified.
• New head SHA on fork: 03c4cbd.

[brendancol]

Rebased onto current main at 7aca800 to pick up the Python 3.14 CI matrix limit (b1579f8).