Remove stale external-overview skips on dask/fsspec/GPU golden-corpus backends#3026
Merged
Conversation
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.
brendancol
commented
Jun 7, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: Remove stale external-overview skips on dask/fsspec/GPU golden-corpus backends
Blockers (must fix before merge)
None.
Suggestions (should fix, not blocking)
None.
Nits (optional improvements)
- test_fsspec.py: the sidecar is served inside
test_fsspec_parityvia thememory_fs_cleanfixture handle, while the base.tifgoes through the module-level_serve_via_memoryhelper. Both write to the same process-global memory filesystem so it works, but a reader scanning the test sees two write paths for related files. Folding the sidecar write into_serve_via_memory(or a small sibling helper) keeps the memory-FS setup in one place. Cosmetic; the current form is correct.
What looks good
- The four skip removals are verified rather than assumed. Each backend's overview-level comparison was run and passes, and the taxonomy guard tests still pass because the entries were removed cleanly, not left dangling.
- The fsspec change is the substantive one and is right. That skip was masking a test-harness gap (only the base
.tifwas in the memory filesystem), not a reader limitation. Serving the sibling.tif.ovrlets the real fsspec sidecar discovery path (_probe_fsspec->load_sidecar) resolve the external overview levels, so the test exercises production code instead of routing around it. - The sidecar path construction (
path.name+.ovr) matchesfind_sidecar'ssource + ".ovr"convention, so the served URL lines up with what the reader probes. - Comments left in place of the removed entries explain why the gap is gone, matching the eager numpy module's existing note.
Checklist
- Algorithm matches reference/paper (n/a, test-only)
- All implemented backends produce consistent results (all four verified passing)
- NaN handling is correct (n/a)
- Edge cases covered by tests (this PR adds the previously-skipped overview coverage)
- Dask chunk boundaries handled correctly (n/a, no production change)
- No premature materialization or unnecessary copies (n/a)
- Benchmark exists or is not needed (not needed, test-only)
- README feature matrix updated (n/a, no new function)
- Docstrings present and accurate (skip-list comments updated)
#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.
brendancol
commented
Jun 7, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review (after edcf7bb)
The single nit from the first pass is resolved. The fsspec sidecar write now lives in _serve_via_memory alongside the base .tif write, so all memory-filesystem setup is in one helper and the test body no longer carries a parallel write path. Full test_fsspec.py module passes (32 passed, 2 skipped).
No new findings. Nothing outstanding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3024
What
The golden-corpus dask, fsspec, GPU, and dask+GPU parity modules listed
overview_external_ovr_uint16in_OVERVIEW_READER_GAPS, which switched off the overview-level oracle comparison and ran only the base-IFD check. The reader has external.ovrsidecar discovery (_sidecar.py) shared across backends, so these gates were stale.overview_external_ovr_uint16entry from_OVERVIEW_READER_GAPSin all four modules. The overview-level comparison now runs and passes..tif.ovrinto thememory://filesystem so the reader's fsspec sidecar discovery (_probe_fsspec->load_sidecar) can resolve the external overview levels. Without it the memory filesystem held only the base.tifandoverview_level>=1failed as out of range. That was a test-harness gap, not a reader limitation.Backends
Test-only change. All four backends (dask+numpy, fsspec, GPU/cupy, dask+GPU) run and pass the overview comparison in the dev environment used here, which has a working CUDA device. Eager numpy already had its gap cleared.
Test plan
test_dask_numpy_parity[overview_external_ovr_uint16]passestest_fsspec_parity[overview_external_ovr_uint16]passestest_gpu_parity[overview_external_ovr_uint16]passestest_dask_gpu_parity[overview_external_ovr_uint16]passestest_taxonomy_ids_are_in_manifestpasses in each module