From 8f194fe30a7fab591aa22549e9eeaaf72bc8ddd3 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Sun, 17 May 2026 20:39:30 -0700 Subject: [PATCH] geotiff: deprecate projected-CRS GeoKey attrs in DataArray.attrs (#1984) The pass-through tier of the attrs contract (#1984, locked in #2004) flagged ``linear_units`` and ``projection_code`` as dropped on a write + read round-trip: ``build_geo_tags`` only emits the primary ``GEOKEY_PROJECTED_CS_TYPE`` and never the secondary projected GeoKeys (``GEOKEY_PROJECTION``, ``GEOKEY_PROJ_LINEAR_UNITS``), so the reader has nothing to rebuild the attrs from. Wrap the two emission sites in ``_populate_attrs_from_geo_info`` so each one fires a ``DeprecationWarning`` referencing #1984. The attrs are still emitted during the deprecation cycle; removal is a follow-up. Docstring: move both keys from the pass-through tier to a new "Deprecated" section. A sibling PR is adding the same section for geographic GeoKey attrs and may collide on this docstring block. Tests: ``test_attrs_pr7_deprecate_projected_1984.py`` covers the two warnings plus a guard that the attrs still populate during the deprecation cycle. --- CHANGELOG.md | 1 + xrspatial/geotiff/_attrs.py | 30 ++- ...test_attrs_pr7_deprecate_projected_1984.py | 210 ++++++++++++++++++ 3 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 xrspatial/geotiff/tests/test_attrs_pr7_deprecate_projected_1984.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ae2b872d..309530f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Unreleased #### Bug fixes and improvements +- Deprecate read-side emission of projected-CRS GeoKey attrs (linear_units, projection_code); the writer cannot reconstruct them so they do not round-trip. These attrs still emit for now but trigger a DeprecationWarning. Removal planned for a future release. (#1984) - Refresh the geotiff mmap cache when a file is replaced under the same path so re-reads after an atomic-rename overwrite no longer return stale bytes - Decode TIFF predictor=3 un-transpose by file byte order so big-endian floating-point TIFFs read back exactly - Default internal `_vrt.read_vrt` `missing_sources` to `'raise'` so an unreadable VRT source no longer produces a silent zero-fill hole on integer rasters; pass `missing_sources='warn'` to opt back into the previous lenient behaviour (#1843) diff --git a/xrspatial/geotiff/_attrs.py b/xrspatial/geotiff/_attrs.py index 865138ae7..462b1bca2 100644 --- a/xrspatial/geotiff/_attrs.py +++ b/xrspatial/geotiff/_attrs.py @@ -65,10 +65,8 @@ - ``geog_citation``: GeographicTypeGeoKey citation string. - ``datum_code``: GeogGeodeticDatumGeoKey value. - ``angular_units``: GeogAngularUnitsGeoKey value. -- ``linear_units``: ProjLinearUnitsGeoKey value. - ``semi_major_axis``: GeogSemiMajorAxisGeoKey value. - ``inv_flattening``: GeogInvFlatteningGeoKey value. -- ``projection_code``: ProjectedCSTypeGeoKey value. - ``vertical_crs``: VerticalCSTypeGeoKey value. - ``vertical_citation``: VerticalCitationGeoKey value. - ``vertical_units``: VerticalUnitsGeoKey value. @@ -76,6 +74,18 @@ - ``extra_samples``: TIFF ExtraSamples tag. - ``colormap``, ``colormap_rgba``, ``cmap``: palette data attached to single-band paletted images. + +Deprecated (will be removed in a future release; see issue #1984): + +- ``linear_units``: ProjLinearUnitsGeoKey value. The writer's + ``build_geo_tags`` only emits the primary ``GEOKEY_PROJECTED_CS_TYPE`` + and never the secondary projected GeoKeys, so this attr cannot be + reconstructed on round-trip. Read-side emission triggers a + ``DeprecationWarning`` for one release cycle before removal. +- ``projection_code``: ProjectionGeoKey value. Same root cause as + ``linear_units``: the writer never emits the underlying GeoKey, so + the value cannot survive a round-trip. Read-side emission triggers a + ``DeprecationWarning`` for one release cycle before removal. """ from __future__ import annotations @@ -248,12 +258,28 @@ def _populate_attrs_from_geo_info(attrs: dict, geo_info, *, window=None) -> None if geo_info.angular_units is not None: attrs['angular_units'] = geo_info.angular_units if geo_info.linear_units is not None: + warnings.warn( + "xrspatial.geotiff: attrs['linear_units'] is deprecated; " + "the writer cannot reconstruct it from the canonical CRS " + "so it will not round-trip. It will be removed in a future " + "release. See issue #1984.", + DeprecationWarning, + stacklevel=2, + ) attrs['linear_units'] = geo_info.linear_units if geo_info.semi_major_axis is not None: attrs['semi_major_axis'] = geo_info.semi_major_axis if geo_info.inv_flattening is not None: attrs['inv_flattening'] = geo_info.inv_flattening if geo_info.projection_code is not None: + warnings.warn( + "xrspatial.geotiff: attrs['projection_code'] is deprecated; " + "the writer cannot reconstruct it from the canonical CRS " + "so it will not round-trip. It will be removed in a future " + "release. See issue #1984.", + DeprecationWarning, + stacklevel=2, + ) attrs['projection_code'] = geo_info.projection_code if geo_info.vertical_epsg is not None: attrs['vertical_crs'] = geo_info.vertical_epsg diff --git a/xrspatial/geotiff/tests/test_attrs_pr7_deprecate_projected_1984.py b/xrspatial/geotiff/tests/test_attrs_pr7_deprecate_projected_1984.py new file mode 100644 index 000000000..e1c107546 --- /dev/null +++ b/xrspatial/geotiff/tests/test_attrs_pr7_deprecate_projected_1984.py @@ -0,0 +1,210 @@ +"""Deprecation tests for the projected-CRS GeoKey pass-through attrs. + +Issue #1984, PR 7 (projected-attrs slice). + +PR 6 (#2004) locked the pass-through tier of the attrs contract and +flagged ``linear_units`` and ``projection_code`` as DROPPED on a write + +read round-trip: ``xrspatial.geotiff._geotags.build_geo_tags`` only +emits the primary ``GEOKEY_PROJECTED_CS_TYPE`` (3072) and never the +secondary projected GeoKeys (``GEOKEY_PROJECTION`` 3074, +``GEOKEY_PROJ_LINEAR_UNITS`` 3076), so the reader has nothing to +rebuild the attrs from. + +This file pins the deprecation behaviour for one release cycle: + +* ``linear_units`` and ``projection_code`` are still emitted by the + read path when the source file carries the secondary GeoKey. +* Each emission triggers a ``DeprecationWarning`` pointing at issue + #1984 so downstream code can migrate before the attrs are removed. + +Removal will land in a follow-up after the deprecation cycle. +""" +from __future__ import annotations + +import struct +import warnings + +import numpy as np +import pytest + +from xrspatial.geotiff import open_geotiff + +from .conftest import make_minimal_tiff + + +def _make_projected_tiff_with_secondary_geokeys( + *, + epsg: int = 32633, + include_linear_units: bool = False, + include_projection_code: bool = False, + linear_units_code: int = 9001, # metre + projection_code_value: int = 16033, +) -> bytes: + """Build a TIFF carrying a projected CRS plus selected secondary GeoKeys. + + Builds on :func:`make_minimal_tiff` to produce a stripped float32 + TIFF with ModelPixelScale + ModelTiepoint and a GeoKeyDirectory + (tag 34735) that carries the primary + ``GEOKEY_PROJECTED_CS_TYPE`` (3072) entry plus optional secondary + ``GEOKEY_PROJECTION`` (3074) and ``GEOKEY_PROJ_LINEAR_UNITS`` + (3076) entries. ``make_minimal_tiff`` itself only emits the primary + GeoKey, so we rebuild and replace tag 34735 in the output bytes. + """ + # Start with the baseline projected TIFF (primary GeoKey only). + base = make_minimal_tiff( + 4, 4, np.dtype('float32'), + geo_transform=(500000.0, 4649776.0, 30.0, -30.0), + epsg=epsg, + ) + + # Compose the full GeoKey directory we want to inject. Format per + # the GeoTIFF spec for tag 34735: header [version, rev_major, + # rev_minor, num_keys] followed by 4-short tuples + # [key_id, tiff_tag_location, count, value_or_offset]. + keys: list[tuple[int, int, int, int]] = [] + # GTModelTypeGeoKey -> Projected + keys.append((1024, 0, 1, 1)) + # GTRasterTypeGeoKey -> PixelIsArea (default but explicit) + keys.append((1025, 0, 1, 1)) + # ProjectedCSTypeGeoKey + keys.append((3072, 0, 1, epsg)) + if include_projection_code: + keys.append((3074, 0, 1, projection_code_value)) + if include_linear_units: + keys.append((3076, 0, 1, linear_units_code)) + + num_keys = len(keys) + gkd: list[int] = [1, 1, 0, num_keys] + for k in keys: + gkd.extend(k) + + # Locate tag 34735 in the original IFD and overwrite its short + # buffer in place. ``make_minimal_tiff`` always uses little-endian + # ('II', magic 42) and a standard 8-byte header, so the IFD starts + # at offset 8 with a uint16 entry count followed by 12-byte + # records. + out = bytearray(base) + ifd_off = struct.unpack_from(' str: + path = tmp_path / name + path.write_bytes(payload) + return str(path) + + +def test_linear_units_emits_deprecation_warning(tmp_path): + """Reading a projected TIFF with ``ProjLinearUnitsGeoKey`` fires + a ``DeprecationWarning`` for ``attrs['linear_units']``.""" + payload = _make_projected_tiff_with_secondary_geokeys( + epsg=32633, include_linear_units=True, + ) + path = _write_tmp_tiff(tmp_path, 'linear_units.tif', payload) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + open_geotiff(path) + + matched = [ + w for w in caught + if issubclass(w.category, DeprecationWarning) + and "attrs['linear_units']" in str(w.message) + ] + assert matched, ( + "expected a DeprecationWarning mentioning attrs['linear_units']; " + f"captured warnings: {[(w.category.__name__, str(w.message)) for w in caught]}" + ) + msg = str(matched[0].message) + assert '#1984' in msg, ( + f"deprecation message should reference issue #1984: {msg!r}" + ) + + +def test_projection_code_emits_deprecation_warning(tmp_path): + """Reading a projected TIFF with ``ProjectionGeoKey`` fires a + ``DeprecationWarning`` for ``attrs['projection_code']``.""" + payload = _make_projected_tiff_with_secondary_geokeys( + epsg=32633, include_projection_code=True, + ) + path = _write_tmp_tiff(tmp_path, 'projection_code.tif', payload) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + open_geotiff(path) + + matched = [ + w for w in caught + if issubclass(w.category, DeprecationWarning) + and "attrs['projection_code']" in str(w.message) + ] + assert matched, ( + "expected a DeprecationWarning mentioning attrs['projection_code']; " + f"captured warnings: {[(w.category.__name__, str(w.message)) for w in caught]}" + ) + msg = str(matched[0].message) + assert '#1984' in msg, ( + f"deprecation message should reference issue #1984: {msg!r}" + ) + + +def test_deprecated_projected_attrs_still_emitted(tmp_path): + """During the deprecation cycle the attrs are still populated. + + Removing emission is a separate follow-up; this test guards + against an accidental early removal. + """ + payload = _make_projected_tiff_with_secondary_geokeys( + epsg=32633, + include_linear_units=True, + include_projection_code=True, + ) + path = _write_tmp_tiff(tmp_path, 'both.tif', payload) + + with warnings.catch_warnings(): + warnings.simplefilter('ignore', DeprecationWarning) + da = open_geotiff(path) + + assert 'linear_units' in da.attrs, ( + "attrs['linear_units'] disappeared during deprecation cycle; " + "emission removal is a separate follow-up to this PR." + ) + assert da.attrs['linear_units'] == 'metre' + + assert 'projection_code' in da.attrs, ( + "attrs['projection_code'] disappeared during deprecation cycle; " + "emission removal is a separate follow-up to this PR." + ) + assert da.attrs['projection_code'] == 16033