Skip to content

Commit 8fcb89e

Browse files
authored
Reject VRT sources under stable_only=True (#2443) (#2447)
* Reject VRT sources under stable_only=True (#2443) Add a `stable_only: bool = False` kwarg to `open_geotiff`, `read_geotiff_dask`, `read_geotiff_gpu`, and `read_vrt`. When True, the VRT path raises a typed `VRTStableSourcesOnlyError` (a `GeoTIFFAmbiguousMetadataError` subclass) before any pixel decode. The message names the offending VRT path and the `allow_experimental_codecs` unlock so callers can opt into the broader tier set explicitly. Flips the `test_release_gate_negative_mixed_tier_vrt_children` xfail from epic #2342. * Address review nits: defensive VRT-extension check, tighter test asserts (#2443) - ``_validate_stable_only_vrt`` now passes through silently when the source is not a ``.vrt`` path, so a future call site that forwards a non-VRT path cannot trip a mislabeled "VRT source" rejection. The docstring spells out the pass-through contract. - ``test_vrt_stable_only_2443.py`` pins the downstream ``VRTUnsupportedError`` on the default-false and ``allow_experimental_codecs``-unlock paths so a future refactor cannot silently broaden either branch past intent.
1 parent fe16afe commit 8fcb89e

10 files changed

Lines changed: 298 additions & 24 deletions

File tree

xrspatial/geotiff/__init__.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@
7676
MixedBandMetadataError,
7777
NonRepresentableEPSGCRSError, NonUniformCoordsError, RotatedTransformError,
7878
UnknownCRSModelTypeError,
79-
UnparseableCRSError, UnsupportedGeoTIFFFeatureError)
79+
UnparseableCRSError, UnsupportedGeoTIFFFeatureError,
80+
VRTStableSourcesOnlyError)
8081
from ._geotags import RASTER_PIXEL_IS_AREA, RASTER_PIXEL_IS_POINT, GeoTransform # noqa: F401
8182
from ._reader import _MAX_CLOUD_BYTES_SENTINEL, CloudSizeLimitError, UnsafeURLError
8283
from ._reader import read_to_array as _read_to_array
@@ -122,6 +123,7 @@
122123
'UnparseableCRSError',
123124
'UnsafeURLError',
124125
'UnsupportedGeoTIFFFeatureError',
126+
'VRTStableSourcesOnlyError',
125127
'open_geotiff',
126128
'read_geotiff_gpu',
127129
'read_geotiff_dask',
@@ -389,6 +391,7 @@ def open_geotiff(source: str | BinaryIO, *,
389391
allow_unparseable_crs: bool = False,
390392
allow_inconsistent_geokeys: bool = False,
391393
allow_invalid_nodata: bool = False,
394+
stable_only: bool = False,
392395
allow_experimental_codecs: bool = False,
393396
allow_internal_only_jpeg: bool = False,
394397
band_nodata: str | None = None,
@@ -611,6 +614,19 @@ def open_geotiff(source: str | BinaryIO, *,
611614
pre-rejection no-op behaviour for files known to carry such
612615
sentinels (e.g. external tooling that writes ``"nan"`` on
613616
integer outputs). See issue #2441 (#1774 follow-up).
617+
stable_only : bool, default False
618+
[advanced] Read-side opt-in for stable-tier sources only. When
619+
``True``, a ``.vrt`` source raises
620+
:class:`VRTStableSourcesOnlyError` because ``reader.vrt`` and
621+
the VRT child-source pipeline sit at the ``advanced`` /
622+
``experimental`` tiers in
623+
:data:`xrspatial.geotiff.SUPPORTED_FEATURES`. Non-VRT sources
624+
on this entry point already ride the stable ``reader.local_file``
625+
path and the per-source codec gate, so the flag is a no-op for
626+
them. The rejection names the file path and the
627+
``allow_experimental_codecs`` opt-in so the caller can unlock
628+
the broader tier set explicitly when needed. See epic #2342
629+
and ``docs/source/reference/release_gate_geotiff.rst``.
614630
allow_experimental_codecs : bool, default False
615631
Read-side opt-in for sources compressed with the Tier 3
616632
experimental codecs (``lerc``, ``jpeg2000`` / ``j2k``, ``lz4``).
@@ -761,6 +777,7 @@ def open_geotiff(source: str | BinaryIO, *,
761777
allow_inconsistent_geokeys=(
762778
allow_inconsistent_geokeys),
763779
allow_invalid_nodata=allow_invalid_nodata,
780+
stable_only=stable_only,
764781
allow_experimental_codecs=allow_experimental_codecs,
765782
allow_internal_only_jpeg=allow_internal_only_jpeg,
766783
band_nodata=band_nodata,
@@ -786,6 +803,7 @@ def open_geotiff(source: str | BinaryIO, *,
786803
allow_inconsistent_geokeys=(
787804
allow_inconsistent_geokeys),
788805
allow_invalid_nodata=allow_invalid_nodata,
806+
stable_only=stable_only,
789807
allow_experimental_codecs=(
790808
allow_experimental_codecs),
791809
allow_internal_only_jpeg=(
@@ -804,6 +822,7 @@ def open_geotiff(source: str | BinaryIO, *,
804822
allow_inconsistent_geokeys=(
805823
allow_inconsistent_geokeys),
806824
allow_invalid_nodata=allow_invalid_nodata,
825+
stable_only=stable_only,
807826
allow_experimental_codecs=(
808827
allow_experimental_codecs),
809828
allow_internal_only_jpeg=(

xrspatial/geotiff/_backends/dask.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def read_geotiff_dask(source: str, *,
4444
allow_unparseable_crs: bool = False,
4545
allow_inconsistent_geokeys: bool = False,
4646
allow_invalid_nodata: bool = False,
47+
stable_only: bool = False,
4748
allow_experimental_codecs: bool = False,
4849
allow_internal_only_jpeg: bool = False,
4950
band_nodata: str | None = None,
@@ -142,6 +143,13 @@ def read_geotiff_dask(source: str, *,
142143
``InvalidIntegerNodataError`` at graph-build time. See
143144
``open_geotiff`` for the full description (#1774 follow-up,
144145
#2441).
146+
stable_only : bool, default False
147+
[advanced] Read-side opt-in for stable-tier sources only.
148+
Forwarded to ``read_vrt`` when the source ends in ``.vrt`` so
149+
the rejection fires at graph-build time. Non-VRT sources on
150+
this entry point already ride the stable ``reader.local_file``
151+
path, so the flag is a no-op for them. See ``open_geotiff`` for
152+
the full description (epic #2342).
145153
allow_experimental_codecs : bool, default False
146154
[advanced] Read-side opt-in for Tier 3 experimental codecs
147155
(``lerc``, ``jpeg2000`` / ``j2k``, ``lz4``). Fires at graph
@@ -227,6 +235,9 @@ def read_geotiff_dask(source: str, *,
227235
allow_unparseable_crs=allow_unparseable_crs,
228236
allow_inconsistent_geokeys=allow_inconsistent_geokeys,
229237
allow_invalid_nodata=allow_invalid_nodata,
238+
stable_only=stable_only,
239+
allow_experimental_codecs=allow_experimental_codecs,
240+
allow_internal_only_jpeg=allow_internal_only_jpeg,
230241
band_nodata=band_nodata,
231242
mask_nodata=mask_nodata,
232243
**vrt_kwargs,

xrspatial/geotiff/_backends/gpu.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ def read_geotiff_gpu(source: str, *,
7777
allow_unparseable_crs: bool = False,
7878
allow_inconsistent_geokeys: bool = False,
7979
allow_invalid_nodata: bool = False,
80+
stable_only: bool = False,
8081
allow_experimental_codecs: bool = False,
8182
allow_internal_only_jpeg: bool = False,
8283
band_nodata: str | None = None,
@@ -217,6 +218,13 @@ def read_geotiff_gpu(source: str, *,
217218
eager and dask paths; default raises
218219
``InvalidIntegerNodataError``. See ``open_geotiff`` for the full
219220
description (#1774 follow-up, #2441).
221+
stable_only : bool, default False
222+
[experimental] Read-side opt-in for stable-tier sources only.
223+
The GPU read path does not consume VRT sources directly (VRT
224+
routing happens in ``open_geotiff``), so this kwarg is accepted
225+
for cross-backend signature symmetry and is a no-op on the GPU
226+
eager / chunked paths. See ``open_geotiff`` for the full
227+
description (epic #2342).
220228
allow_experimental_codecs : bool, default False
221229
[experimental] Read-side opt-in for Tier 3 experimental codecs
222230
(``lerc``, ``jpeg2000`` / ``j2k``, ``lz4``). The GPU read path

xrspatial/geotiff/_backends/vrt.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ def read_vrt(source: str, *,
130130
allow_unparseable_crs: bool = False,
131131
allow_inconsistent_geokeys: bool = False,
132132
allow_invalid_nodata: bool = False,
133+
stable_only: bool = False,
133134
allow_experimental_codecs: bool = False,
134135
allow_internal_only_jpeg: bool = False,
135136
band_nodata: str | None = None,
@@ -276,6 +277,17 @@ def read_vrt(source: str, *,
276277
the per-source GeoTIFF reads built by the VRT planner. See
277278
``open_geotiff`` for the full description (#1774 follow-up,
278279
#2441).
280+
stable_only : bool, default False
281+
[advanced] Read-side opt-in for stable-tier sources only. When
282+
``True``, ``read_vrt`` raises :class:`VRTStableSourcesOnlyError`
283+
before any pixel decode because ``reader.vrt`` itself sits at
284+
the ``advanced`` tier in :data:`SUPPORTED_FEATURES` and VRT
285+
child sources can declare any codec the GeoTIFF reader supports
286+
(including experimental and internal-only tiers). The message
287+
names the file path and the ``allow_experimental_codecs``
288+
unlock so the caller can opt into the broader tier set
289+
explicitly. See epic #2342 and
290+
``docs/source/reference/release_gate_geotiff.rst``.
279291
allow_experimental_codecs : bool, default False
280292
[advanced] Read-side opt-in for Tier 3 experimental codecs in
281293
any source file referenced by the VRT. Forwarded to the
@@ -373,6 +385,22 @@ def read_vrt(source: str, *,
373385

374386
source = _coerce_path(source)
375387

388+
# Epic #2342: reject the read up front when the caller asked for
389+
# stable-only sources. ``reader.vrt`` sits at the ``advanced`` tier
390+
# and VRT children can declare any codec the GeoTIFF reader
391+
# supports, so a stable-only request cannot be served from a VRT
392+
# mosaic without the documented ``allow_experimental_codecs``
393+
# unlock. Runs before the dispatcher-kwarg validator so the typed
394+
# error surfaces before any other validation noise (a malformed VRT
395+
# path, an unsupported ``overview_level``, etc.) competes for the
396+
# raise site.
397+
from .._validation import _validate_stable_only_vrt
398+
_validate_stable_only_vrt(
399+
source,
400+
stable_only=stable_only,
401+
allow_experimental_codecs=allow_experimental_codecs,
402+
)
403+
376404
# Shared dispatcher-kwarg validator so direct callers see the same
377405
# rejections as ``open_geotiff`` (issue #2175 / parent #2162). For
378406
# ``read_vrt`` the helper rejects ``on_gpu_failure`` (VRT reads do

xrspatial/geotiff/_errors.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,26 @@ class InvalidIntegerNodataError(GeoTIFFAmbiguousMetadataError):
166166
"""
167167

168168

169+
class VRTStableSourcesOnlyError(GeoTIFFAmbiguousMetadataError):
170+
"""VRT source opened under ``stable_only=True`` (epic #2342).
171+
172+
Raised when a caller opens a ``.vrt`` file with ``stable_only=True``.
173+
The VRT reader (``reader.vrt``) and its child sources sit at the
174+
``advanced`` / ``experimental`` tiers in
175+
:data:`xrspatial.geotiff.SUPPORTED_FEATURES`, so a request for
176+
stable-only sources cannot be served from a VRT mosaic without an
177+
explicit opt-in. The message names the offending VRT path and the
178+
matching opt-in flag (``allow_experimental_codecs``) so the caller
179+
learns the unlock at the boundary rather than from the docs.
180+
181+
Pass ``stable_only=False`` (the default) to keep the legacy
182+
behaviour, or pass ``allow_experimental_codecs=True`` to opt into
183+
the broader tier set explicitly. See the release contract document
184+
at ``docs/source/reference/release_gate_geotiff.rst`` and epic
185+
#2342 for the full rationale.
186+
"""
187+
188+
169189
class UnknownCRSModelTypeError(GeoTIFFAmbiguousMetadataError):
170190
"""Can't classify an EPSG as geographic or projected on write (#2277).
171191
@@ -230,6 +250,7 @@ class UnsupportedGeoTIFFFeatureError(ValueError):
230250
"ConflictingCRSError",
231251
"ConflictingNodataError",
232252
"InvalidIntegerNodataError",
253+
"VRTStableSourcesOnlyError",
233254
"VRTUnsupportedError",
234255
"UnknownCRSModelTypeError",
235256
"NonRepresentableEPSGCRSError",

xrspatial/geotiff/_validation.py

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from ._coords import _BAND_DIM_NAMES
2929
from ._errors import (ConflictingCRSError, ConflictingNodataError, InconsistentGeoKeysError,
3030
InvalidIntegerNodataError, MixedBandMetadataError, NonUniformCoordsError,
31-
RotatedTransformError, UnparseableCRSError)
31+
RotatedTransformError, UnparseableCRSError, VRTStableSourcesOnlyError)
3232
from ._runtime import (_MISSING_SOURCES_SENTINEL, _ON_GPU_FAILURE_SENTINEL, _TIME_DIM_NAMES,
3333
_X_DIM_NAMES, _Y_DIM_NAMES)
3434

@@ -668,6 +668,83 @@ def _validate_int_nodata_for_dtype(
668668
)
669669

670670

671+
def _validate_stable_only_vrt(
672+
source: str,
673+
*,
674+
stable_only: bool,
675+
allow_experimental_codecs: bool = False,
676+
) -> None:
677+
"""Reject a VRT source when the caller asks for stable-only sources.
678+
679+
Implements the read-side gate the release-contract test
680+
``test_release_gate_negative_mixed_tier_vrt_children`` pins from
681+
epic #2342. The ``reader.vrt`` entry point sits at the ``advanced``
682+
tier in :data:`xrspatial.geotiff.SUPPORTED_FEATURES`, and VRT child
683+
sources can declare any codec / capability the underlying GeoTIFF
684+
reader supports (including the ``experimental`` and ``internal_only``
685+
tiers). A caller who asks for stable-only sources via
686+
``stable_only=True`` therefore cannot be served from a VRT mosaic
687+
without naming the broader-tier opt-in (``allow_experimental_codecs``).
688+
Reject up front at graph-build / eager-read setup time so the
689+
failure surfaces before any pixel decode work.
690+
691+
Parameters
692+
----------
693+
source : str
694+
Path to the ``.vrt`` file. Embedded in the rejection message so
695+
the caller can locate the offending file without re-parsing.
696+
Non-string sources (eager file-like buffers) and string paths
697+
that do not end in ``.vrt`` are silently passed through; the
698+
gate only fires for sources the caller could reasonably have
699+
intended as a VRT mosaic, so a future call site that routes a
700+
non-VRT path through this helper does not mislabel the failure.
701+
stable_only : bool
702+
Caller's opt-in for stable-only sources. ``False`` is a no-op;
703+
``True`` raises :class:`VRTStableSourcesOnlyError` (provided
704+
``source`` looks like a VRT path).
705+
allow_experimental_codecs : bool, default False
706+
Companion opt-in. When the caller passes both ``stable_only=True``
707+
and ``allow_experimental_codecs=True`` the request is internally
708+
contradictory, but ``allow_experimental_codecs=True`` is the
709+
documented unlock so we honour it: the gate becomes a no-op and
710+
the per-source codec gate downstream handles the rest. Pure
711+
``stable_only=True`` (without the unlock) raises.
712+
713+
Raises
714+
------
715+
VRTStableSourcesOnlyError
716+
When ``stable_only=True`` and the source path ends in ``.vrt``
717+
(case-insensitive) and the caller did not pass
718+
``allow_experimental_codecs=True``. The message names the
719+
offending VRT path, both flags, and cites the release-contract
720+
document plus epic #2342.
721+
"""
722+
if not stable_only:
723+
return
724+
if allow_experimental_codecs:
725+
return
726+
# Defensive extension check: every public-API call site routes only
727+
# ``.vrt`` paths into this helper, but a future call site could
728+
# forward a non-VRT path. Pass through silently in that case so the
729+
# rejection message never mislabels a non-VRT source as a VRT.
730+
if not (isinstance(source, str) and source.lower().endswith('.vrt')):
731+
return
732+
raise VRTStableSourcesOnlyError(
733+
f"VRT source '{source}' cannot be opened under stable_only=True. "
734+
f"The VRT reader (``reader.vrt``) sits at the advanced tier in "
735+
f"SUPPORTED_FEATURES, and VRT child sources can declare any "
736+
f"codec or capability the GeoTIFF reader supports, including "
737+
f"the experimental and internal-only tiers. The stable-only "
738+
f"request cannot be served from a VRT mosaic without an "
739+
f"explicit broader-tier opt-in. Pass "
740+
f"allow_experimental_codecs=True to opt in to the advanced / "
741+
f"experimental tiers, or drop stable_only=True to keep the "
742+
f"default behaviour. See "
743+
f"docs/source/reference/release_gate_geotiff.rst for the "
744+
f"release contract and epic #2342 for the tracking issue."
745+
)
746+
747+
671748
def _validate_no_rotated_affine(attrs, *, drop_rotation: bool,
672749
entry_point: str = "to_geotiff") -> None:
673750
"""Refuse writes that would silently drop ``attrs['rotated_affine']``.

xrspatial/geotiff/tests/release_gates/test_stable_features.py

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,31 +2415,16 @@ def test_release_gate_negative_rotated_gpu(
24152415

24162416

24172417
@pytest.mark.release_gate
2418-
@pytest.mark.xfail(
2419-
reason=(
2420-
"The VRT stable-only knob is owned by epic #2342 and has not "
2421-
"landed yet. The release promise: when the caller asks for "
2422-
"stable-only sources and a VRT child uses an experimental codec, "
2423-
"the reader names the offending child and the opt-in flag. This "
2424-
"xfail flips to a pass when #2342 ships the knob."
2425-
),
2426-
strict=False,
2427-
)
24282418
def test_release_gate_negative_mixed_tier_vrt_children(tmp_path) -> None:
24292419
"""The reader must refuse mixed-tier VRT children when stable-only is asked.
24302420
2431-
XFAIL-to-PASS transition note
2432-
-----------------------------
2433-
Today this test fails with ``TypeError: unexpected keyword argument
2434-
'stable_only'`` because epic #2342 has not landed the kwarg yet. The
2435-
strict=False xfail swallows that TypeError. When #2342 lands, the
2436-
test will start raising :class:`GeoTIFFAmbiguousMetadataError` (or
2437-
fail to raise) and the xfail will report XPASS. Before removing the
2438-
xfail marker, confirm the new code path satisfies both inline
2439-
assertions: the error message must mention either ``stable_only`` or
2440-
``allow_experimental_codecs``, and it must cite the release contract
2441-
docs. If either assertion would not pass, fix the production message
2442-
in the same PR that removes the xfail.
2421+
Pinned by epic #2342 / issue #2443: when the caller asks for
2422+
stable-only sources via ``stable_only=True`` and the source is a
2423+
VRT, the read raises :class:`VRTStableSourcesOnlyError` (a
2424+
:class:`GeoTIFFAmbiguousMetadataError` subclass) before any pixel
2425+
decode. The message must name either ``stable_only`` or
2426+
``allow_experimental_codecs`` and cite the release-contract docs
2427+
or the tracking issue.
24432428
"""
24442429
path = _neg_tmp(tmp_path, "case4_mixed_tier_vrt", suffix=".vrt")
24452430
Path(path).write_text(

xrspatial/geotiff/tests/test_features.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2804,6 +2804,11 @@ def test_all_lists_supported_functions(self):
28042804
# pansharpened / derived VRT subclasses, unknown VRT band
28052805
# children, rotated source transforms on a VRT mosaic).
28062806
'UnsupportedGeoTIFFFeatureError',
2807+
# Issue #2443 (epic #2342): typed rejection when a caller
2808+
# opens a VRT under ``stable_only=True``. The VRT reader
2809+
# itself is advanced-tier so the request cannot be served
2810+
# without naming the broader-tier opt-in.
2811+
'VRTStableSourcesOnlyError',
28072812
'GeoTIFFFallbackWarning',
28082813
'UnsafeURLError',
28092814
# Canonical georef_status constants (issue #2136). Exposed

xrspatial/geotiff/tests/test_reader_kwarg_order_1935.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@
4747
# opt-outs so the canonical order keeps the typed-error gates
4848
# grouped.
4949
"allow_invalid_nodata",
50+
# Issue #2443 (epic #2342) added the stable-tier-only read-side
51+
# gate. Sits alongside the other ambiguous-metadata opt-outs and
52+
# immediately before the experimental-codec unlock it pairs with
53+
# in the rejection message, so the canonical order tracks the
54+
# release-contract grouping.
55+
"stable_only",
5056
# PR 4 of epic #2340 added the experimental / internal-only codec
5157
# opt-ins on the read side, mirroring the writer surface from #2137
5258
# / #1845. They sit after the other ``allow_*`` flags so the

0 commit comments

Comments
 (0)