geotiff: activate mixed-band-metadata fail-closed check (#1987 PR 5)#2090
Merged
Conversation
Registers ``_check_read_mixed_band_metadata`` so VRT reads whose bands declare disagreeing per-band ``<NoDataValue>`` sentinels raise ``MixedBandMetadataError`` by default. The check and its ``band_nodata=`` opt-out were staged in #2031 but not registered: the activation needed the existing VRT test sites that mosaic distinct per-band sentinels to migrate to ``band_nodata='first'`` first. Wires the opt-out into ``open_geotiff`` and ``read_geotiff_dask`` so all three VRT entry points expose the same kwarg, with a clear ValueError on non-VRT sources (same dispatch-kwarg guard pattern as ``missing_sources`` / ``on_gpu_failure`` / ``max_cloud_bytes``). Migrates the four VRT test files whose fixtures intentionally mosaic bands with distinct sentinels: - ``test_vrt_band_nodata_1598.py`` - ``test_vrt_multiband_int_nodata_1611.py`` - ``test_vrt_int_source_float_dtype_1616.py`` - ``test_vrt_multiband_dtype_1696.py`` Each migrated call adds ``band_nodata='first'`` so the regression keeps asserting the legacy flatten-to-first-band semantics, now via the documented opt-out instead of implicitly. Adds ``test_mixed_band_metadata_fail_closed_1987.py`` with 11 cases pinning the fail-closed contract end-to-end: default raise, chunked raise, opt-out passthrough, shared-sentinel acceptance, one-band-only acceptance, ``open_geotiff`` / ``read_geotiff_dask`` propagation, and the non-VRT-source rejection on both entry points. Refs #1987.
read_vrt accepted any string for band_nodata, but the registered mixed-band-metadata check (issue #1987 PR 5) only treats 'first' as the opt-out and silently falls back to strict mode for anything else. A typo like band_nodata='firs' looked like an explicit opt-out at the call site but raised MixedBandMetadataError at run time. Reject unknown values at the read_vrt entry point so the error surfaces where the typo lives, matching the missing_sources value-validation pattern already in this function. open_geotiff and read_geotiff_dask route through read_vrt for VRT sources, so one validation site covers all three public entry points. Also fills in the read_vrt docstring entry for band_nodata, which had been left undocumented since #2031 staged the kwarg. Refs #1987.
brendancol
force-pushed
the
1987-pr5-mixed-band-metadata
branch
from
5c6d2ec to
7dda360
Compare
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
1 participant
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.
Summary
Closes the last open slice of #1987. The check function
_check_read_mixed_band_metadataand theband_nodata=opt-out onread_vrtshipped in #2031 but were not registered, becauseactivation needed a coordinated migration of the VRT test fixtures that
mosaic bands with distinct per-band sentinels.
<NoDataValue>sentinels raiseMixedBandMetadataErrorbydefault.
band_nodata=throughopen_geotiffandread_geotiff_daskand rejects the kwarg on non-VRT sources witha clear ValueError (matches
missing_sources/on_gpu_failure/max_cloud_bytesdispatch-kwarg guards).distinct per-band sentinels (
test_vrt_band_nodata_1598,test_vrt_multiband_int_nodata_1611,test_vrt_int_source_float_dtype_1616,test_vrt_multiband_dtype_1696) to passband_nodata='first'.The opt-out keeps the legacy flatten-to-first-band semantics each
regression covers; the explicit kwarg now surfaces that intent at the
call site.
test_mixed_band_metadata_fail_closed_1987.pywith 11 casespinning the end-to-end contract: default raise on the eager and
chunked paths, opt-out passthrough, shared-sentinel acceptance,
one-band-only-declares acceptance,
open_geotiffandread_geotiff_daskpropagation, base-class catch, and the non-VRTrejection.
PR list status (#1987)
crs/crs_wkton writeThis PR closes #1987 once merged.
Test plan
pytest xrspatial/geotiff/tests/test_mixed_band_metadata_fail_closed_1987.py -v-- 11 passed.pytest xrspatial/geotiff/tests/(excluding two pre-existing unrelated failures intest_predictor2_big_endian_gpu_1517.pyandtest_size_param_validation_gpu_vrt_1776.py) -- 3868 passed, 25 skipped.pytest xrspatial/geotiff/tests/test_ambiguous_metadata_hooks_1987.py-- 26 passed (registry framework unaffected).pytest xrspatial/geotiff/tests/test_reader_kwarg_order_1935.py-- 5 passed (band_nodataadded to_CANONICAL_ORDERbetweenallow_unparseable_crsandmask_nodata).