From e7607983f6bd1bece74a547db30033003be27503 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Sun, 7 Jun 2026 09:24:42 -0700 Subject: [PATCH 1/2] Remove stale external-overview skips on dask/fsspec/GPU backends (#3024) The golden-corpus dask, fsspec, GPU, and dask+GPU modules listed overview_external_ovr_uint16 in _OVERVIEW_READER_GAPS, which skipped the overview-level oracle comparison. The reader gained external .ovr sidecar discovery (_sidecar.py) shared across backends, so these gates were stale. Drop the entry from all four modules. For fsspec, also push the sibling .tif.ovr into the memory filesystem so the reader's fsspec sidecar discovery can resolve the external overview levels; without it the memory filesystem held only the base .tif and the test failed as a harness gap, not a reader limitation. --- .../tests/golden_corpus/test_dask_gpu.py | 11 +++++----- .../tests/golden_corpus/test_dask_numpy.py | 11 +++++----- .../tests/golden_corpus/test_fsspec.py | 20 ++++++++++++------- .../geotiff/tests/golden_corpus/test_gpu.py | 11 +++++----- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/xrspatial/geotiff/tests/golden_corpus/test_dask_gpu.py b/xrspatial/geotiff/tests/golden_corpus/test_dask_gpu.py index 9d0939a37..4f31e2277 100644 --- a/xrspatial/geotiff/tests/golden_corpus/test_dask_gpu.py +++ b/xrspatial/geotiff/tests/golden_corpus/test_dask_gpu.py @@ -82,11 +82,12 @@ # xrspatial reader. The base-IFD parity check still runs; the # overview-level loop driven by ``candidate_factory`` is skipped for # these ids. See the eager module for the rationale. -_OVERVIEW_READER_GAPS: dict[str, str] = { - "overview_external_ovr_uint16": ( - "External .ovr sidecar reader is not implemented in xrspatial." - ), -} +# +# Empty now that the dask+GPU reader resolves the sibling ``.tif.ovr`` +# pyramid through the shared ``_sidecar`` discovery the eager numpy +# backend uses, so the overview-level comparison runs for +# ``overview_external_ovr_uint16`` on this backend too. +_OVERVIEW_READER_GAPS: dict[str, str] = {} _INTENTIONAL_SKIPS: dict[str, str] = { "nodata_miniswhite_uint8": ( diff --git a/xrspatial/geotiff/tests/golden_corpus/test_dask_numpy.py b/xrspatial/geotiff/tests/golden_corpus/test_dask_numpy.py index 1a80b71e4..38c68cc39 100644 --- a/xrspatial/geotiff/tests/golden_corpus/test_dask_numpy.py +++ b/xrspatial/geotiff/tests/golden_corpus/test_dask_numpy.py @@ -78,11 +78,12 @@ # xrspatial reader. The base-IFD parity check still runs; the # overview-level loop driven by ``candidate_factory`` is skipped for # these ids. See the eager module for the rationale. -_OVERVIEW_READER_GAPS: dict[str, str] = { - "overview_external_ovr_uint16": ( - "External .ovr sidecar reader is not implemented in xrspatial." - ), -} +# +# Empty now that the dask reader resolves the sibling ``.tif.ovr`` +# pyramid through the shared ``_sidecar`` discovery the eager numpy +# backend uses, so the overview-level comparison runs for +# ``overview_external_ovr_uint16`` on this backend too. +_OVERVIEW_READER_GAPS: dict[str, str] = {} _INTENTIONAL_SKIPS: dict[str, str] = { "nodata_miniswhite_uint8": ( diff --git a/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py b/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py index 252a0d842..e7bac1d6a 100644 --- a/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py +++ b/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py @@ -84,13 +84,7 @@ # xrspatial reader. The base-IFD parity check still runs; the # overview-level loop driven by ``candidate_factory`` is skipped for # these ids. See the eager module for the rationale. -_OVERVIEW_READER_GAPS: dict[str, str] = { - "overview_external_ovr_uint16": ( - "External .ovr sidecar reader is not implemented in xrspatial. " - "The base IFD still parity-checks; the overview levels live in " - "a sibling .tif.ovr that the reader does not open." - ), -} +_OVERVIEW_READER_GAPS: dict[str, str] = {} _INTENTIONAL_SKIPS: dict[str, str] = { "nodata_miniswhite_uint8": ( @@ -203,6 +197,18 @@ def test_fsspec_parity(manifest_entry: dict, memory_fs_clean) -> None: payload = f.read() url = _serve_via_memory(payload, fixture_id) + # When the fixture ships a sibling ``.tif.ovr`` sidecar, push it + # into the memory filesystem under the matching URL so the reader's + # fsspec sidecar discovery (``_probe_fsspec`` -> ``load_sidecar``) + # can resolve the external overview levels. Without this the memory + # filesystem holds only the base ``.tif`` and ``overview_level>=1`` + # fails as out of range, which is a test-harness gap rather than a + # reader limitation. + sidecar_path = path.parent / f"{path.name}.ovr" + if sidecar_path.exists(): + with open(sidecar_path, "rb") as f: + memory_fs_clean.pipe(f"/corpus/{fixture_id}.tif.ovr", f.read()) + candidate = open_geotiff(url, **_OPTIN) # When the fixture carries pyramid overviews, hand the oracle a # factory it can use to fetch each overview level via the same diff --git a/xrspatial/geotiff/tests/golden_corpus/test_gpu.py b/xrspatial/geotiff/tests/golden_corpus/test_gpu.py index e94581032..54d5612df 100644 --- a/xrspatial/geotiff/tests/golden_corpus/test_gpu.py +++ b/xrspatial/geotiff/tests/golden_corpus/test_gpu.py @@ -84,11 +84,12 @@ # xrspatial reader. The base-IFD parity check still runs; the # overview-level loop driven by ``candidate_factory`` is skipped for # these ids. See the eager module for the rationale. -_OVERVIEW_READER_GAPS: dict[str, str] = { - "overview_external_ovr_uint16": ( - "External .ovr sidecar reader is not implemented in xrspatial." - ), -} +# +# Empty now that the GPU reader resolves the sibling ``.tif.ovr`` +# pyramid through the shared ``_sidecar`` discovery the eager numpy +# backend uses, so the overview-level comparison runs for +# ``overview_external_ovr_uint16`` on this backend too. +_OVERVIEW_READER_GAPS: dict[str, str] = {} # Fixtures whose codec is not implemented on the GPU read path and # legitimately need to fall back to CPU. The test routes these through From edcf7bb46de06c65ce3afa85aa9343b69a995998 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Sun, 7 Jun 2026 09:26:32 -0700 Subject: [PATCH 2/2] Address review nit: fold fsspec sidecar serving into _serve_via_memory (#3024) Keep all memory-FS writes in one helper instead of splitting the base .tif and sibling .tif.ovr across the helper and the test body. --- .../tests/golden_corpus/test_fsspec.py | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py b/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py index e7bac1d6a..c13113820 100644 --- a/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py +++ b/xrspatial/geotiff/tests/golden_corpus/test_fsspec.py @@ -146,6 +146,13 @@ def _serve_via_memory(payload: bytes, fixture_id: str) -> str: Returns the ``memory:///corpus/.tif`` URL the reader should open. The three-slash form is required: ``memory://`` writes must use a path beginning with ``/`` or fsspec rejects them. + + When the fixture ships a sibling ``.tif.ovr`` sidecar on disk, push + it under the matching ``.tif.ovr`` URL so the reader's fsspec + sidecar discovery (``_probe_fsspec`` -> ``load_sidecar``) can resolve + the external overview levels. Without it the memory filesystem holds + only the base ``.tif`` and ``overview_level>=1`` fails as out of + range, which is a test-harness gap rather than a reader limitation. """ fs = fsspec.filesystem("memory") url = f"memory:///corpus/{fixture_id}.tif" @@ -153,6 +160,9 @@ def _serve_via_memory(payload: bytes, fixture_id: str) -> str: # filesystems; ``test_cloud_read_byte_limit_1928.py`` uses the # same pattern. fs.pipe(f"/corpus/{fixture_id}.tif", payload) + sidecar_path = FIXTURES_DIR / f"{fixture_id}.tif.ovr" + if sidecar_path.exists(): + fs.pipe(f"/corpus/{fixture_id}.tif.ovr", sidecar_path.read_bytes()) return url @@ -197,18 +207,6 @@ def test_fsspec_parity(manifest_entry: dict, memory_fs_clean) -> None: payload = f.read() url = _serve_via_memory(payload, fixture_id) - # When the fixture ships a sibling ``.tif.ovr`` sidecar, push it - # into the memory filesystem under the matching URL so the reader's - # fsspec sidecar discovery (``_probe_fsspec`` -> ``load_sidecar``) - # can resolve the external overview levels. Without this the memory - # filesystem holds only the base ``.tif`` and ``overview_level>=1`` - # fails as out of range, which is a test-harness gap rather than a - # reader limitation. - sidecar_path = path.parent / f"{path.name}.ovr" - if sidecar_path.exists(): - with open(sidecar_path, "rb") as f: - memory_fs_clean.pipe(f"/corpus/{fixture_id}.tif.ovr", f.read()) - candidate = open_geotiff(url, **_OPTIN) # When the fixture carries pyramid overviews, hand the oracle a # factory it can use to fetch each overview level via the same