Gate dict gdal_metadata behind the experimental rich-tag opt-in (#3327)

Author: brendancol

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). |
 

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:

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.

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 {}

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
 
 

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
 
 

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
 
 

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'

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")

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': '<GDALMetadata/>',
+             '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': '<GDALMetadata/>',
-         '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': '<GDALMetadata/>',
          'extra_tags': [(700, 1, 0, b'')],
+         'gdal_metadata': {'AREA_OR_POINT': 'Area'},
          '_xrspatial_geotiff_contract': 2},
         allow_experimental_codecs=False,
     )