geotiff: chunked-VRT coverage for missing_sources policy (#1799)

[brendancol]

Summary

Closes a Cat 1 MEDIUM cross-backend equivalence gap for issue #1799 (VRT missing_sources policy). test_vrt_missing_sources_policy_1799.py covers the eager read_vrt path but not the chunked path.

The chunked path (_backends/vrt.py:_read_vrt_chunked) plumbs missing_sources through two separate code paths:

1. A parse-time static os.path.exists sweep at vrt.py:595-621 that populates attrs['vrt_holes'] at build time without forcing a compute (the contract documented in geotiff: read_vrt default lenient mode hides source read failures as zero-filled holes #1734 for partial-mosaic detection on the lazy graph).
2. Per-chunk forwarding of missing_sources to _vrt_chunk_read so each task applies the same warn/raise policy as the eager path.

A regression dropping the parse-time sweep would break partial-mosaic detection on the lazy build; a regression dropping the per-chunk forward would silently flip missing_sources='raise' to 'warn' (or vice versa) on the chunked path.

8 new tests, all passing:

• missing_sources='warn' populates vrt_holes at build + emits warning on compute + present-source chunk decodes correctly
• chunks=(h, w) tuple form threads through identically
• missing_sources='raise' fails at compute on chunks touching missing sources but succeeds on windowed compute that misses them
• default missing_sources on the chunked path is 'raise' (parity with eager default flipped in VRT missing_sources default before stable release #1843 / geotiff: read_vrt and open_geotiff(.vrt) default missing_sources to lenient 'warn' #1860)
• Invalid policy raises ValueError at build, parity with eager path

Mutation against the attrs['vrt_holes'] population at vrt.py:620-621 flipped the two build-time tests red.

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

Test plan

• pytest xrspatial/geotiff/tests/test_vrt_chunked_missing_sources_1799.py (8 passed)
• pytest xrspatial/geotiff/tests/test_vrt_chunked_missing_sources_1799.py xrspatial/geotiff/tests/test_vrt_missing_sources_policy_1799.py (11 passed)
• Mutation: drop attrs['vrt_holes'] = chunked_holes at vrt.py:620 -> 2 build-time 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: chunked-VRT coverage for missing_sources policy (#1799)

Blockers (must fix before merge)

• None

Suggestions (should fix, not blocking)

• test_vrt_chunked_missing_sources_1799.py:96-99 only checks that holes[0]['source'] ends with missing.tif. The full record schema (band, dst_rect, error) is documented at _backends/vrt.py:608 and a regression dropping any of those fields would not be caught. Asserting on the dict keys (or the band and dst_rect values) would pin the schema the eager path also returns.
• test_vrt_chunked_missing_sources_1799.py:120-123 (test_compute_emits_per_task_warning) pins the present-side decode to 7.0 but does not assert anything about the missing-side fill. The PR description says it decodes to "typically zero for float32" but the test silently allows any value. If the lenient path stops zero-filling and returns garbage on the missing half, this test would still pass. Either pin the missing-side fill or drop the comment.
• test_compute_intersecting_missing_raises and test_chunked_default_raises_on_compute catch (OSError, FileNotFoundError, ValueError) which is loose. The eager-path equivalent at test_vrt_missing_sources_policy_1799.py:41 catches (OSError, ValueError). If you want parity with the eager test, drop FileNotFoundError (it is already a subclass of OSError).

Nits (optional improvements)

• test_vrt_chunked_missing_sources_1799.py:55-75 builds the VRT XML by string concatenation. The sibling helper in test_vrt_chunked_shared_dataset_1923.py does the same, so this is consistent, but an f-string template or textwrap.dedent would be easier to read.
• test_vrt_chunked_missing_sources_1799.py:141 comment says "2 chunks vertically * 2 chunks horizontally = 4 tasks" but the assertion is only on len(vrt_holes) == 1. The 4-task claim is unverified; either assert on result.data.numblocks or drop the comment.
• _make_partial_vrt returns (vrt_path, present_src_path) but every call site discards the second element with _. Returning just vrt_path would simplify the signature.

What looks good

• Test rationale is documented in module docstring and per-class docstrings, including the parse-time vs decode-time split and the geotiff: read_vrt default lenient mode hides source read failures as zero-filled holes #1734 / VRT missing_sources default before stable release #1843 / geotiff: read_vrt and open_geotiff(.vrt) default missing_sources to lenient 'warn' #1860 cross-references.
• Mutation evidence in the PR body (dropping attrs['vrt_holes'] = chunked_holes at vrt.py:620 flips 2 tests red) gives concrete bite for the parse-time sweep.
• Uses unique tmp filenames via tmp_path, matching the project workflow note.
• Covers both chunks=int and chunks=(h, w) tuple forms, plus the windowed case where the missing source is excluded from the graph.
• Test-only PR with no source modifications; risk surface is bounded to the test suite.

Checklist

• Algorithm matches reference/paper (n/a, test-only)
• All implemented backends produce consistent results (test pins cross-mode parity between eager and chunked)
• NaN handling is correct (n/a for missing-source policy)
• Edge cases are covered by tests (warn, raise, default, invalid policy, tuple chunks, windowed)
• Dask chunk boundaries handled correctly (test exercises chunks that do and do not intersect missing sources)
• No premature materialization or unnecessary copies (test explicitly checks compute() is deferred)
• Benchmark exists or is not needed (n/a, test-only)
• README feature matrix updated (if applicable) (n/a, test-only)
• Docstrings present and accurate

[brendancol]

PR Review: geotiff: chunked-VRT coverage for missing_sources policy (#1799) (re-review after rebase)

Blockers (must fix before merge)

• None

Suggestions (should fix, not blocking)

• test_vrt_chunked_missing_sources_1799.py:160 and :197 catch (OSError, ValueError). The eager path's contract on missing_sources='raise' is more specific than that. If the reader raises a FileNotFoundError (subclass of OSError) on missing files today, pinning that subclass would catch a regression where the raise switches to a generic RuntimeError or Exception. Consider narrowing once the exact exception type is confirmed against the eager test in test_vrt_missing_sources_policy_1799.py.
• test_vrt_chunked_missing_sources_1799.py:130-141 (test_chunks_tuple_form): the assertion only checks len(result.attrs["vrt_holes"]) == 1. The chunks tuple form is meant to thread through identically, so asserting the chunk shape on result.data (e.g. result.data.chunks == ((2, 2), (4, 4))) would pin the actual tuple-form behavior the docstring claims.

Nits (optional improvements)

• test_vrt_chunked_missing_sources_1799.py:39 (_make_partial_vrt): could be a pytest fixture rather than a helper function. Minor; the helper is clear enough as is.
• test_vrt_chunked_missing_sources_1799.py:57-76: the VRT XML is built by string concatenation. A small helper or textwrap.dedent with a triple-quoted template would read more cleanly, but the current form is fine.

What looks good

• Each test class targets one invariant (warn/raise/default/validation) with a focused docstring that explains why the contract matters.
• Both sides of the mosaic are asserted in test_compute_emits_per_task_warning (:125-128): present-source fill value pinned and missing-source NaN pinned.
• test_compute_present_only_chunk_succeeds (:163) covers the window scoping rule, which is a real edge case.
• vrt_holes record schema is pinned at :103 ({"source", "band", "dst_rect", "error"}), which will catch silent key renames in the parse-time sweep.
• Parity test at :209 confirms the eager and chunked paths reject the same invalid policy value.

Checklist

• Algorithm matches reference/paper (test-only PR; pins existing behavior)
• All implemented backends produce consistent results (eager vs chunked parity pinned)
• NaN handling is correct (lenient missing source -> NaN asserted at :128)
• Edge cases are covered by tests (window scoping, tuple chunks, invalid policy)
• Dask chunk boundaries handled correctly (chunks=4 splits at the missing-source boundary)
• No premature materialization or unnecessary copies (vrt_holes checked before .compute())
• Benchmark exists or is not needed (N/A - test only)
• README feature matrix updated (if applicable) (N/A)
• Docstrings present and accurate

[brendancol]

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