diff --git a/docs/source/reference/geotiff_release_contract.md b/docs/source/reference/geotiff_release_contract.md index 69063af23..0938c1c9b 100644 --- a/docs/source/reference/geotiff_release_contract.md +++ b/docs/source/reference/geotiff_release_contract.md @@ -106,7 +106,7 @@ category. The `Key` column matches the runtime key. | `writer.bigtiff` | advanced | `bigtiff=True` (or auto-promotion above 4 GiB) writes BigTIFF magic, 8-byte offsets, and 20-byte IFD entries. | | `writer.bigtiff_cog` | advanced | BigTIFF plus COG. Tracked separately because the combination has its own external-interop surface. | | `writer.gpu` | experimental | GPU write path. | -| `writer.gdal_metadata_xml` | experimental | `attrs['gdal_metadata_xml']` is escaped before serialisation. | +| `writer.gdal_metadata_xml` | experimental | `attrs['gdal_metadata_xml']` is escaped before serialisation. A dict `attrs['gdal_metadata']` rides the same gate: writers build the XML from it, so a fresh DataArray carrying it also requires `allow_experimental_codecs=True` (#3320). | | `writer.extra_tags` | experimental | Pass-through of TIFF tags outside the structured set via `attrs['extra_tags']`. | | `writer.pack` | experimental | `pack=True` on `to_geotiff`; inverse of `reader.unpack`. Re-applies the recorded scale/offset, restores the integer source dtype, and fills NaN back to the nodata sentinel. Tracked at the same tier as `reader.unpack` (#3112, #3114). | diff --git a/xrspatial/geotiff/_attrs.py b/xrspatial/geotiff/_attrs.py index b20d18331..47faa2515 100644 --- a/xrspatial/geotiff/_attrs.py +++ b/xrspatial/geotiff/_attrs.py @@ -531,6 +531,11 @@ def _validate_write_rich_tag_optin( triggered: list[str] = [] if attrs.get('gdal_metadata_xml') is not None: triggered.append("attrs['gdal_metadata_xml']") + # ``_extract_rich_tags`` only builds GDAL XML from ``gdal_metadata`` + # when it is a dict (and ``gdal_metadata_xml`` is absent); a non-dict + # value is ignored by the writer, so it does not need gating. + if isinstance(attrs.get('gdal_metadata'), dict): + triggered.append("attrs['gdal_metadata']") if attrs.get('extra_tags') is not None: triggered.append("attrs['extra_tags']") if gdal_metadata_xml_kwarg is not None: diff --git a/xrspatial/geotiff/tests/attrs/test_contract.py b/xrspatial/geotiff/tests/attrs/test_contract.py index 67a0d6210..1be15b548 100644 --- a/xrspatial/geotiff/tests/attrs/test_contract.py +++ b/xrspatial/geotiff/tests/attrs/test_contract.py @@ -347,6 +347,49 @@ def test_canonical_keys_present_per_backend(tmp_path, opener, label): ) +def _make_gdal_metadata_da(): + """Fresh DataArray (no contract marker) carrying only a dict + ``gdal_metadata``. The writer turns this into on-disk GDAL XML, so it + must trip the experimental rich-tag gate (#3320).""" + data = np.arange(4, dtype=np.float32).reshape(2, 2) + return xr.DataArray( + data, dims=('y', 'x'), + coords={'y': [240.0, 230.0], 'x': [100.0, 110.0]}, + attrs={'crs': 4326, 'gdal_metadata': {'AREA_OR_POINT': 'Area'}}, + ) + + +def test_gdal_metadata_dict_requires_optin_3320(tmp_path): + """A fresh DataArray whose only rich-tag attr is a dict + ``gdal_metadata`` writes GDAL XML to disk, so ``to_geotiff`` must + reject it without ``allow_experimental_codecs`` and accept it with + the flag (#3320).""" + da = _make_gdal_metadata_da() + rejected = str(tmp_path / 'gdal_metadata_optin_3320_rejected.tif') + with pytest.raises(ValueError, match=r"attrs\['gdal_metadata'\]"): + to_geotiff(da, rejected) + assert not os.path.exists(rejected) + + accepted = str(tmp_path / 'gdal_metadata_optin_3320_accepted.tif') + to_geotiff(da, accepted, allow_experimental_codecs=True) + assert os.path.exists(accepted) + + +def test_gdal_metadata_dict_roundtrip_writes_flag_free_3320(tmp_path): + """A read-back DataArray carries the contract marker, so writing its + ``gdal_metadata`` back is the canonical round-trip and stays + flag-free even without the opt-in (#3320).""" + da = _make_gdal_metadata_da() + src = str(tmp_path / 'gdal_metadata_roundtrip_3320_src.tif') + to_geotiff(da, src, allow_experimental_codecs=True) + + rd = open_geotiff(src) + assert '_xrspatial_geotiff_contract' in rd.attrs + out = str(tmp_path / 'gdal_metadata_roundtrip_3320_out.tif') + to_geotiff(rd, out) + assert os.path.exists(out) + + # ``raster_type`` lives outside the shared fixture because the canonical # default ('area') is encoded as *absence* in attrs. The two branches # need different fixtures. diff --git a/xrspatial/geotiff/tests/gpu/test_writer.py b/xrspatial/geotiff/tests/gpu/test_writer.py index 8341b70d3..ac93edba5 100644 --- a/xrspatial/geotiff/tests/gpu/test_writer.py +++ b/xrspatial/geotiff/tests/gpu/test_writer.py @@ -301,7 +301,9 @@ def test_gdal_metadata_round_trips_via_gpu_writer(tmp_path): 'CUSTOM_KEY': 'val_1563'}}, ) out = str(tmp_path / 'gdal_meta_1563.tif') - _write_geotiff_gpu(da_gpu, out, compression='none') + # gdal_metadata dict is an experimental rich-tag write (#3320). + _write_geotiff_gpu(da_gpu, out, compression='none', + allow_experimental_codecs=True) rd = open_geotiff(out) meta = rd.attrs.get('gdal_metadata') or {} diff --git a/xrspatial/geotiff/tests/read/test_mask_and_scale_dtype_parity_3066.py b/xrspatial/geotiff/tests/read/test_mask_and_scale_dtype_parity_3066.py index 402798304..5d1cb70e5 100644 --- a/xrspatial/geotiff/tests/read/test_mask_and_scale_dtype_parity_3066.py +++ b/xrspatial/geotiff/tests/read/test_mask_and_scale_dtype_parity_3066.py @@ -23,12 +23,15 @@ def _write(path, data, *, nodata=None, scale=None, offset=None): "SCALE": str(scale if scale is not None else 1.0), "OFFSET": str(offset if offset is not None else 0.0), } + # gdal_metadata SCALE/OFFSET dict on a fresh array is an experimental + # rich-tag write (#3320). to_geotiff( xr.DataArray( data, dims=("y", "x"), coords={"y": np.arange(h, 0, -1) - 0.5, "x": np.arange(w) + 0.5}, attrs=attrs), - path) + path, + allow_experimental_codecs="gdal_metadata" in attrs) return path diff --git a/xrspatial/geotiff/tests/read/test_rioxarray_compat_2961.py b/xrspatial/geotiff/tests/read/test_rioxarray_compat_2961.py index 603bc20b5..8f3266c74 100644 --- a/xrspatial/geotiff/tests/read/test_rioxarray_compat_2961.py +++ b/xrspatial/geotiff/tests/read/test_rioxarray_compat_2961.py @@ -48,7 +48,9 @@ def _scale_offset_tiff(path, scale=2.0, offset=10.0, sentinel=255): "gdal_metadata": {"SCALE": str(scale), "OFFSET": str(offset)}, }, ) - to_geotiff(da, path) + # gdal_metadata dict on a fresh array is an experimental rich-tag + # write (#3320). + to_geotiff(da, path, allow_experimental_codecs=True) return path @@ -261,7 +263,9 @@ def _malformed_scale_tiff(path, scale="abc", offset="0"): "gdal_metadata": {"SCALE": scale, "OFFSET": offset}, }, ) - to_geotiff(da, path) + # gdal_metadata dict on a fresh array is an experimental rich-tag + # write (#3320). + to_geotiff(da, path, allow_experimental_codecs=True) return path @@ -486,7 +490,9 @@ def _per_band_scale_tiff(path, scales, offsets): }, attrs={"crs": 4326, "gdal_metadata": meta}, ) - to_geotiff(da, path) + # gdal_metadata dict on a fresh array is an experimental rich-tag + # write (#3320). + to_geotiff(da, path, allow_experimental_codecs=True) return path diff --git a/xrspatial/geotiff/tests/read/test_scale_zero_3104.py b/xrspatial/geotiff/tests/read/test_scale_zero_3104.py index e992fd295..cff62ef7c 100644 --- a/xrspatial/geotiff/tests/read/test_scale_zero_3104.py +++ b/xrspatial/geotiff/tests/read/test_scale_zero_3104.py @@ -38,7 +38,9 @@ def _scale_tiff(path, scale, offset="0", nodata=None): coords={"y": [1.5, 0.5], "x": [0.5, 1.5, 2.5]}, attrs=attrs, ) - to_geotiff(da, path) + # gdal_metadata dict on a fresh array is an experimental rich-tag + # write (#3320). + to_geotiff(da, path, allow_experimental_codecs=True) return path diff --git a/xrspatial/geotiff/tests/release_gates/test_features.py b/xrspatial/geotiff/tests/release_gates/test_features.py index 0ad8d5b46..1e471e5e6 100644 --- a/xrspatial/geotiff/tests/release_gates/test_features.py +++ b/xrspatial/geotiff/tests/release_gates/test_features.py @@ -1519,7 +1519,8 @@ def test_dataarray_attrs_round_trip(self, tmp_path): attrs={'gdal_metadata': meta}, ) path = str(tmp_path / 'da_meta.tif') - to_geotiff(da, path, compression='none') + # gdal_metadata dict is an experimental rich-tag write (#3320). + to_geotiff(da, path, compression='none', allow_experimental_codecs=True) result = open_geotiff(path) assert result.attrs['gdal_metadata']['Source'] == 'test' diff --git a/xrspatial/geotiff/tests/unit/test_safe_xml.py b/xrspatial/geotiff/tests/unit/test_safe_xml.py index 12290072d..d5d3bdc73 100644 --- a/xrspatial/geotiff/tests/unit/test_safe_xml.py +++ b/xrspatial/geotiff/tests/unit/test_safe_xml.py @@ -116,7 +116,8 @@ def test_to_geotiff_special_chars_round_trip(self, tmp_path): } path = tmp_path / "meta.tif" - to_geotiff(da, str(path)) + # gdal_metadata dict is an experimental rich-tag write (#3320). + to_geotiff(da, str(path), allow_experimental_codecs=True) opened = open_geotiff(str(path)) round_tripped = opened.attrs.get("gdal_metadata") diff --git a/xrspatial/geotiff/tests/unit/test_signatures.py b/xrspatial/geotiff/tests/unit/test_signatures.py index 73eb4c0f4..832523eb2 100644 --- a/xrspatial/geotiff/tests/unit/test_signatures.py +++ b/xrspatial/geotiff/tests/unit/test_signatures.py @@ -762,11 +762,48 @@ def test_validate_write_rich_tag_optin_rejects_extra_tags(): ) +def test_validate_write_rich_tag_optin_rejects_gdal_metadata_dict(): + """A dict ``attrs['gdal_metadata']`` is written to disk as XML by + ``_extract_rich_tags``; it must trip the same gate (#3320). + """ + with pytest.raises(ValueError, match=r"attrs\['gdal_metadata'\]"): + _validate_write_rich_tag_optin( + {'gdal_metadata': {'AREA_OR_POINT': 'Area'}}, + allow_experimental_codecs=False, + ) + + +def test_validate_write_rich_tag_optin_names_both_xml_and_gdal_metadata(): + """When both ``gdal_metadata_xml`` and a dict ``gdal_metadata`` are + present without the opt-in, the rejection names both attrs (#3320).""" + with pytest.raises(ValueError) as exc: + _validate_write_rich_tag_optin( + {'gdal_metadata_xml': '', + 'gdal_metadata': {'AREA_OR_POINT': 'Area'}}, + allow_experimental_codecs=False, + ) + msg = str(exc.value) + assert "attrs['gdal_metadata_xml']" in msg + assert "attrs['gdal_metadata']" in msg + + +def test_validate_write_rich_tag_optin_ignores_non_dict_gdal_metadata(): + """Only a dict ``gdal_metadata`` builds XML in ``_extract_rich_tags``; + a non-dict value is ignored by the writer, so the gate stays quiet + (#3320). + """ + _validate_write_rich_tag_optin( + {'gdal_metadata': 'not-a-dict'}, + allow_experimental_codecs=False, + ) + + def test_validate_write_rich_tag_optin_accepts_with_flag(): - """``allow_experimental_codecs=True`` accepts both rich-tag attrs.""" + """``allow_experimental_codecs=True`` accepts all rich-tag attrs.""" _validate_write_rich_tag_optin( {'gdal_metadata_xml': '', - 'extra_tags': [(700, 1, 0, b'')]}, + 'extra_tags': [(700, 1, 0, b'')], + 'gdal_metadata': {'AREA_OR_POINT': 'Area'}}, allow_experimental_codecs=True, ) @@ -780,6 +817,7 @@ def test_validate_write_rich_tag_optin_exempts_round_trip(): _validate_write_rich_tag_optin( {'gdal_metadata_xml': '', 'extra_tags': [(700, 1, 0, b'')], + 'gdal_metadata': {'AREA_OR_POINT': 'Area'}, '_xrspatial_geotiff_contract': 2}, allow_experimental_codecs=False, ) diff --git a/xrspatial/geotiff/tests/vrt/test_metadata.py b/xrspatial/geotiff/tests/vrt/test_metadata.py index 0181a4bd6..765141bef 100644 --- a/xrspatial/geotiff/tests/vrt/test_metadata.py +++ b/xrspatial/geotiff/tests/vrt/test_metadata.py @@ -831,7 +831,8 @@ class TestVrtTiledMetadataParity: def test_nodatavals_alias_propagates_to_tiles(self, tmp_path): da = _tiled_metadata_make_rioxarray_style() vrt = str(tmp_path / 'nodatavals.vrt') - to_geotiff(da, vrt, tile_size=16) + # gdal_metadata dict is an experimental rich-tag write (#3320). + to_geotiff(da, vrt, tile_size=16, allow_experimental_codecs=True) tile_da = open_geotiff(_tiled_metadata_first_tile_path(vrt)) assert tile_da.attrs.get('nodata') == -9999.0 @@ -847,7 +848,7 @@ def test_fill_value_alias_propagates_to_tiles(self, tmp_path): def test_gdal_metadata_propagates_to_tiles(self, tmp_path): da = _tiled_metadata_make_rioxarray_style() vrt = str(tmp_path / 'gdal_meta.vrt') - to_geotiff(da, vrt, tile_size=16) + to_geotiff(da, vrt, tile_size=16, allow_experimental_codecs=True) tile_da = open_geotiff(_tiled_metadata_first_tile_path(vrt)) gm = tile_da.attrs.get('gdal_metadata') assert gm == {'AREA_OR_POINT': 'Area', 'foo': 'bar'} @@ -855,7 +856,7 @@ def test_gdal_metadata_propagates_to_tiles(self, tmp_path): def test_resolution_tags_propagate_to_tiles(self, tmp_path): da = _tiled_metadata_make_rioxarray_style() vrt = str(tmp_path / 'resolution.vrt') - to_geotiff(da, vrt, tile_size=16) + to_geotiff(da, vrt, tile_size=16, allow_experimental_codecs=True) tile_da = open_geotiff(_tiled_metadata_first_tile_path(vrt)) assert tile_da.attrs.get('x_resolution') == 96.0 assert tile_da.attrs.get('y_resolution') == 96.0 @@ -864,7 +865,7 @@ def test_resolution_tags_propagate_to_tiles(self, tmp_path): def test_raster_type_point_propagates_to_tiles(self, tmp_path): da = _tiled_metadata_make_rioxarray_style() vrt = str(tmp_path / 'point.vrt') - to_geotiff(da, vrt, tile_size=16) + to_geotiff(da, vrt, tile_size=16, allow_experimental_codecs=True) tile_da = open_geotiff(_tiled_metadata_first_tile_path(vrt)) assert tile_da.attrs.get('raster_type') == 'point' @@ -873,8 +874,8 @@ def test_tif_vs_vrt_tile_metadata_parity(self, tmp_path): da = _tiled_metadata_make_rioxarray_style() tif_path = str(tmp_path / 'parity.tif') vrt_path = str(tmp_path / 'parity.vrt') - to_geotiff(da, tif_path, tile_size=16) - to_geotiff(da, vrt_path, tile_size=16) + to_geotiff(da, tif_path, tile_size=16, allow_experimental_codecs=True) + to_geotiff(da, vrt_path, tile_size=16, allow_experimental_codecs=True) tif_da = open_geotiff(tif_path) tile_da = open_geotiff(_tiled_metadata_first_tile_path(vrt_path)) keys = ('nodata', 'gdal_metadata', 'raster_type', 'x_resolution', 'y_resolution', 'resolution_unit') # noqa: E501 @@ -934,7 +935,8 @@ def test_nodatavals_alias_dask(self, tmp_path): da_np = xr.DataArray(arr, dims=('y', 'x'), coords={'y': np.arange(8.0), 'x': np.arange(8.0)}, attrs={'nodatavals': (-9999.0,), 'crs': 4326, 'gdal_metadata': {'k': 'v'}}) # noqa: E501 da = xr.DataArray(dska.from_array(arr, chunks=4), dims=da_np.dims, coords=da_np.coords, attrs=da_np.attrs) # noqa: E501 vrt = str(tmp_path / 'dask.vrt') - to_geotiff(da, vrt, tile_size=16) + # gdal_metadata dict is an experimental rich-tag write (#3320). + to_geotiff(da, vrt, tile_size=16, allow_experimental_codecs=True) tile_da = open_geotiff(_tiled_metadata_first_tile_path(vrt)) assert tile_da.attrs.get('nodata') == -9999.0 assert tile_da.attrs.get('gdal_metadata') == {'k': 'v'} diff --git a/xrspatial/geotiff/tests/write/test_pack_3064.py b/xrspatial/geotiff/tests/write/test_pack_3064.py index a27ddd3dd..e594137fb 100644 --- a/xrspatial/geotiff/tests/write/test_pack_3064.py +++ b/xrspatial/geotiff/tests/write/test_pack_3064.py @@ -41,7 +41,9 @@ def _write_int_tiff(path, data, *, nodata=None, scale=None, offset=None): coords={"y": np.arange(h, 0, -1) - 0.5, "x": np.arange(w) + 0.5}, attrs=attrs, ) - to_geotiff(da, path) + # A gdal_metadata SCALE/OFFSET dict on a fresh array is an + # experimental rich-tag write (#3320). + to_geotiff(da, path, allow_experimental_codecs="gdal_metadata" in attrs) return path diff --git a/xrspatial/geotiff/tests/write/test_pack_band_subset_3161.py b/xrspatial/geotiff/tests/write/test_pack_band_subset_3161.py index 0610982ec..45960fb75 100644 --- a/xrspatial/geotiff/tests/write/test_pack_band_subset_3161.py +++ b/xrspatial/geotiff/tests/write/test_pack_band_subset_3161.py @@ -33,7 +33,9 @@ def _write_two_band_tiff(path, *, gdal_metadata, nodata=65535): coords={"y": [1.5, 0.5], "x": [0.5, 1.5, 2.5]}, attrs={"crs": 4326, "nodata": nodata, "gdal_metadata": gdal_metadata}, ) - to_geotiff(da, path) + # gdal_metadata dict on a fresh array is an experimental rich-tag + # write (#3320). + to_geotiff(da, path, allow_experimental_codecs=True) return path, data diff --git a/xrspatial/geotiff/tests/write/test_pack_float_width_3080.py b/xrspatial/geotiff/tests/write/test_pack_float_width_3080.py index e4e1fb862..cca9a9163 100644 --- a/xrspatial/geotiff/tests/write/test_pack_float_width_3080.py +++ b/xrspatial/geotiff/tests/write/test_pack_float_width_3080.py @@ -39,7 +39,9 @@ def _write_f32_tiff(path, data, *, nodata=None, scale=None, offset=None): coords={"y": np.arange(h, 0, -1) - 0.5, "x": np.arange(w) + 0.5}, attrs=attrs, ) - to_geotiff(da, path) + # A gdal_metadata SCALE/OFFSET dict on a fresh array is an + # experimental rich-tag write (#3320). + to_geotiff(da, path, allow_experimental_codecs="gdal_metadata" in attrs) return path diff --git a/xrspatial/geotiff/tests/write/test_pack_lazy_nan_guard_3235.py b/xrspatial/geotiff/tests/write/test_pack_lazy_nan_guard_3235.py index c94d681b1..5e04e5de9 100644 --- a/xrspatial/geotiff/tests/write/test_pack_lazy_nan_guard_3235.py +++ b/xrspatial/geotiff/tests/write/test_pack_lazy_nan_guard_3235.py @@ -28,7 +28,9 @@ def _write_packed_no_sentinel_tiff(path, data, *, scale="0.1", offset="5.0"): attrs={"crs": 4326, "gdal_metadata": {"SCALE": scale, "OFFSET": offset}}, ) - to_geotiff(da, str(path)) + # gdal_metadata dict on a fresh array is an experimental rich-tag + # write (#3320). + to_geotiff(da, str(path), allow_experimental_codecs=True) return str(path) diff --git a/xrspatial/geotiff/tests/write/test_pack_nodata_kwarg_3168.py b/xrspatial/geotiff/tests/write/test_pack_nodata_kwarg_3168.py index 94f2f0f2a..17d71c56f 100644 --- a/xrspatial/geotiff/tests/write/test_pack_nodata_kwarg_3168.py +++ b/xrspatial/geotiff/tests/write/test_pack_nodata_kwarg_3168.py @@ -36,7 +36,9 @@ def _write_int_tiff(path, data, *, nodata=None, scale=None, offset=None): coords={"y": np.arange(h, 0, -1) - 0.5, "x": np.arange(w) + 0.5}, attrs=attrs, ) - to_geotiff(da, path) + # A gdal_metadata SCALE/OFFSET dict on a fresh array is an + # experimental rich-tag write (#3320). + to_geotiff(da, path, allow_experimental_codecs="gdal_metadata" in attrs) return path diff --git a/xrspatial/geotiff/tests/write/test_pack_range_guard_3260.py b/xrspatial/geotiff/tests/write/test_pack_range_guard_3260.py index 7b65e4c69..f181f8329 100644 --- a/xrspatial/geotiff/tests/write/test_pack_range_guard_3260.py +++ b/xrspatial/geotiff/tests/write/test_pack_range_guard_3260.py @@ -33,7 +33,8 @@ def _write_scaled_int16(path, *, nodata=None): coords={"y": [1.5, 0.5], "x": [0.5, 1.5]}, attrs=attrs, ) - to_geotiff(da, str(path), nodata=nodata) + to_geotiff(da, str(path), nodata=nodata, + allow_experimental_codecs=True) return str(path) @@ -79,7 +80,7 @@ def test_pack_rejects_underflow_unsigned(tmp_path, chunks): "gdal_metadata": {"SCALE": "0.5", "OFFSET": "0.0"}}, ) src = str(tmp_path / "src_u16_underflow_3260.tif") - to_geotiff(da, src) + to_geotiff(da, src, allow_experimental_codecs=True) mod = _unpacked(src) mod.data[0, 0] = -1.0 # packs to -2, below uint16 min @@ -127,7 +128,7 @@ def test_pack_accepts_full_dtype_range(tmp_path, chunks): "gdal_metadata": {"SCALE": "0.1", "OFFSET": "0.0"}}, ) src = str(tmp_path / "src_bounds_3260.tif") - to_geotiff(da, src) + to_geotiff(da, src, allow_experimental_codecs=True) mod = _unpacked(src) if chunks is not None: @@ -163,7 +164,8 @@ def test_pack_float_target_not_range_guarded(tmp_path): "gdal_metadata": {"SCALE": "2.0", "OFFSET": "0.0"}}, ) src = str(tmp_path / "src_float_3260.tif") - to_geotiff(da, src, nodata=-9999.0) + to_geotiff(da, src, nodata=-9999.0, + allow_experimental_codecs=True) mod = _unpacked(src) mod.data[0, 0] = 1e30