geotiff: deprecate geographic-CRS GeoKey attrs in DataArray.attrs (#1984)

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Unreleased
 
 #### Bug fixes and improvements
+- Deprecate read-side emission of geographic-CRS GeoKey attrs (crs_name, geog_citation, datum_code, angular_units, semi_major_axis, inv_flattening); 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)
 - 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)
 - Deprecate read-side emission of vertical-CRS GeoKey attrs (vertical_crs, vertical_citation, vertical_units); the writer does not emit vertical GeoKeys 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

docs/source/user_guide/attrs_contract.rst

@@ -223,6 +223,50 @@ present on the original file may be absent after write→read if the
 canonical CRS does not carry enough information to rebuild it.
 
 
+Deprecated GeoKey attrs (issue #1984)
+=====================================
+
+The following attrs are still populated on read for one release
+cycle but each emission fires a ``DeprecationWarning``. The writer's
+``build_geo_tags`` only emits the primary CRS GeoKey and citation for
+each axis (geographic, projected, vertical), so the secondary GeoKeys
+these attrs derive from are never written and the values do not
+survive a write→read round-trip. Migrate to ``crs`` / ``crs_wkt`` and
+derive any needed value with :mod:`pyproj`.
+
+Geographic-CRS GeoKey attrs: ``crs_name``, ``geog_citation``,
+``datum_code``, ``angular_units``, ``semi_major_axis``,
+``inv_flattening``.
+
+Projected-CRS GeoKey attrs: ``linear_units``, ``projection_code``.
+
+Vertical-CRS GeoKey attrs: ``vertical_crs``, ``vertical_citation``,
+``vertical_units``.
+
+Migration recipe::
+
+    from pyproj import CRS
+    crs = CRS.from_wkt(attrs['crs_wkt'])  # or CRS.from_epsg(attrs['crs'])
+
+    # Geographic
+    crs.name                                 # crs_name
+    crs.datum.to_epsg()                      # datum_code
+    crs.ellipsoid.semi_major_metre           # semi_major_axis
+    crs.ellipsoid.inverse_flattening         # inv_flattening
+    # geog_citation / angular_units: best-effort derive from
+    # ``crs`` / ``crs.axis_info``; the original GeoKey citation text
+    # is not generally recoverable.
+
+    # Projected
+    crs.coordinate_system.axis_list[0].unit_name   # linear_units
+    crs.to_epsg()                                  # projection_code
+
+    # Vertical
+    crs.sub_crs_list[-1].to_epsg()                 # vertical_crs
+    crs.sub_crs_list[-1].name                      # vertical_citation
+    crs.sub_crs_list[-1].axis_info[0].unit_name    # vertical_units
+
+
 Versioning
 ==========
 

xrspatial/geotiff/_attrs.py

@@ -61,35 +61,75 @@
 Best-effort pass-through (preserved when the writer can reconstruct
 from canonical state, otherwise dropped on round-trip):
 
+- ``image_description``: TIFF ImageDescription tag.
+- ``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):
+
+These attrs are still emitted on read for one release cycle, but each
+emission triggers a ``DeprecationWarning``. The writer's
+``build_geo_tags`` only emits the primary CRS GeoKey and citation for
+each CRS axis (horizontal / projected / vertical), so the secondary
+GeoKeys these attrs derive from are never written and the values do
+not survive a write -> read round-trip. Callers should stop relying
+on them and use ``crs`` / ``crs_wkt`` instead.
+
+Geographic-CRS GeoKey attrs:
+
 - ``crs_name``: human-readable CRS name from the GeoKey directory.
 - ``geog_citation``: GeographicTypeGeoKey citation string.
 - ``datum_code``: GeogGeodeticDatumGeoKey value.
 - ``angular_units``: GeogAngularUnitsGeoKey value.
 - ``semi_major_axis``: GeogSemiMajorAxisGeoKey value.
 - ``inv_flattening``: GeogInvFlatteningGeoKey value.
-- ``image_description``: TIFF ImageDescription tag.
-- ``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):
+Projected-CRS GeoKey attrs:
 
 - ``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.
+  reconstructed on round-trip.
 - ``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.
+  the value cannot survive a round-trip.
+
+Vertical-CRS GeoKey attrs:
+
 - ``vertical_crs``: VerticalCSTypeGeoKey value. The writer never emits
-  the vertical GeoKey block, so this attr cannot round-trip. It still
-  appears on read but triggers a ``DeprecationWarning``.
+  the vertical GeoKey block, so this attr cannot round-trip.
 - ``vertical_citation``: VerticalCitationGeoKey value. Same deprecation
   reason as ``vertical_crs``.
 - ``vertical_units``: VerticalUnitsGeoKey value. Same deprecation reason
   as ``vertical_crs``.
+
+Migration recipe (the canonical replacement is ``crs`` / ``crs_wkt``
+plus a one-liner with :mod:`pyproj` when a derived value is needed)::
+
+    from pyproj import CRS
+    crs = CRS.from_wkt(attrs['crs_wkt'])  # or CRS.from_epsg(attrs['crs'])
+
+    # Geographic
+    crs.name                                 # crs_name
+    crs.datum.to_epsg()                      # datum_code
+    crs.ellipsoid.semi_major_metre           # semi_major_axis
+    crs.ellipsoid.inverse_flattening         # inv_flattening
+    # geog_citation / angular_units: best-effort derive from
+    # ``crs`` / ``crs.axis_info``; the original GeoKey citation text
+    # is not generally recoverable.
+
+    # Projected
+    crs.coordinate_system.axis_list[0].unit_name   # linear_units
+    crs.to_epsg()                                  # projection_code
+
+    # Vertical
+    crs.sub_crs_list[-1].to_epsg()                 # vertical_crs
+    crs.sub_crs_list[-1].name                      # vertical_citation
+    crs.sub_crs_list[-1].axis_info[0].unit_name    # vertical_units
+
+See ``docs/source/user_guide/attrs_contract.rst`` for the full
+migration notes.
 """
 from __future__ import annotations
 
@@ -139,6 +179,159 @@
 _RESOLUTION_UNIT_IDS = {'none': 1, 'inch': 2, 'centimeter': 3}
 
 
+# Geographic-CRS GeoKey-derived attrs scheduled for removal (issue #1984
+# PR 7). The writer's ``build_geo_tags`` only emits the primary
+# GEOKEY_GEOGRAPHIC_TYPE, never the secondary geographic GeoKeys these
+# attrs are derived from. The values therefore never round-trip. Keep
+# emitting them for one release cycle so external callers can migrate,
+# then drop the emission entirely.
+_DEPRECATED_GEOGRAPHIC_GEOKEY_ATTRS = (
+    'crs_name',
+    'geog_citation',
+    'datum_code',
+    'angular_units',
+    'semi_major_axis',
+    'inv_flattening',
+)
+
+
+# Per-category reason clauses spliced into the deprecation warning by
+# :func:`_emit_deprecated_geokey_attr`. Kept here so the wording stays
+# in lockstep across the three GeoKey-axis tiers (geographic, projected,
+# vertical) and so the test suite can match the canonical strings
+# verbatim.
+_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS = (
+    "the writer cannot reconstruct it from the canonical CRS"
+)
+_GEOKEY_DEPRECATION_REASON_VERTICAL_CRS = (
+    "the writer cannot reconstruct vertical-CRS GeoKeys"
+)
+
+
+def _deprecated_geokey_warning(name: str, *, reason: str) -> str:
+    """Warning text for a deprecated GeoKey-derived attr.
+
+    ``reason`` is the per-category clause that explains why the value
+    will not round-trip; the rest of the message is fixed so callers
+    only have to keep track of the short reason string. The wording is
+    pinned by ``test_warning_message_format`` (geographic tier) and by
+    sibling tests for the projected / vertical tiers, so any tweak
+    here needs to land alongside an update to those tests.
+    """
+    return (
+        f"xrspatial.geotiff: attrs[{name!r}] is deprecated; {reason} "
+        f"so it will not round-trip. It will be removed in a future "
+        f"release. See issue #1984."
+    )
+
+
+def _deprecated_geographic_geokey_warning(name: str) -> str:
+    """Warning text shared by every deprecated geographic-GeoKey attr.
+
+    Thin shim over :func:`_deprecated_geokey_warning` that fixes the
+    reason clause to the geographic-tier wording. Retained so existing
+    callers (notably the unit tests that pin the canonical wording)
+    keep working unchanged.
+    """
+    return _deprecated_geokey_warning(
+        name, reason=_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS,
+    )
+
+
+def _stacklevel_to_external_caller() -> int:
+    """Return a ``stacklevel`` that points the warning at the first frame
+    outside :mod:`xrspatial.geotiff`.
+
+    A fixed ``stacklevel`` is brittle here because the call chain to
+    ``warnings.warn`` differs by backend:
+
+    * numpy path: ``open_geotiff`` -> emit helper -> ``warn`` (3 frames).
+    * dask path: ``open_geotiff`` -> ``read_geotiff_dask`` ->
+      ``_populate_attrs_from_geo_info`` -> emit helper -> ``warn`` (5
+      frames).
+    * direct callers of ``read_geotiff_dask`` / ``read_to_array`` (used
+      internally and in tests) shorten the chain by one.
+
+    Walk the stack from the warn-site outward and stop at the first
+    frame whose module is not ``xrspatial.geotiff*``. Returning a value
+    one greater than the index of that frame matches
+    :func:`warnings.warn` semantics (level 1 = the warn line itself).
+
+    Today the warnings are :class:`DeprecationWarning`, which Python
+    silences by default for library code; the stacklevel mostly affects
+    the test suite. Get it right now so a future change to a louder
+    category (e.g. :class:`FutureWarning`) does not surface the warning
+    as if it came from ``_attrs.py``.
+    """
+    import sys
+
+    # Frame 0 is this function; frame 1 is the warn-site (the caller of
+    # this helper). Start the search at frame 1 so the returned level
+    # maps directly to the ``stacklevel`` argument passed to
+    # ``warnings.warn`` inside the warn-site.
+    frame = sys._getframe(1)
+    level = 1
+    while frame is not None:
+        mod = frame.f_globals.get('__name__', '')
+        is_internal = (
+            mod == 'xrspatial.geotiff'
+            or (mod.startswith('xrspatial.geotiff.')
+                and not mod.startswith('xrspatial.geotiff.tests'))
+        )
+        if not is_internal:
+            return level
+        frame = frame.f_back
+        level += 1
+    # Fell off the top of the stack without finding an external caller;
+    # fall back to a value that at least skips the warn-site itself.
+    return 2
+
+
+def _emit_deprecated_geokey_attr(attrs: dict, name: str, value,
+                                 *, reason: str) -> None:
+    """Emit a deprecated GeoKey-derived attr with a ``DeprecationWarning``.
+
+    Generic helper shared by the geographic, projected, and vertical
+    deprecation tiers (issue #1984 PR 7). ``reason`` is the per-category
+    clause that explains why the value will not round-trip; it is
+    spliced into the warning text by :func:`_deprecated_geokey_warning`.
+    Use :data:`_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS` for the
+    geographic and projected tiers (both lose the value because the
+    writer cannot reconstruct it from the canonical CRS) and
+    :data:`_GEOKEY_DEPRECATION_REASON_VERTICAL_CRS` for the vertical
+    tier (the writer skips the entire vertical GeoKey block).
+
+    Sets ``attrs[name] = value`` after the warning so the read-side
+    emission stays alive for one release cycle and external callers
+    have time to migrate to ``crs`` / ``crs_wkt``.
+
+    The ``stacklevel`` is computed by walking past every
+    ``xrspatial.geotiff*`` frame so the warning is attributed to the
+    user's call site (e.g. ``open_geotiff(...)``) rather than to one of
+    the internal read paths.
+    """
+    warnings.warn(
+        _deprecated_geokey_warning(name, reason=reason),
+        DeprecationWarning,
+        stacklevel=_stacklevel_to_external_caller(),
+    )
+    attrs[name] = value
+
+
+def _emit_deprecated_geographic_geokey(attrs: dict, name: str, value) -> None:
+    """Geographic-tier wrapper around :func:`_emit_deprecated_geokey_attr`.
+
+    Kept as a thin shim so the geographic emission sites in
+    :func:`_populate_attrs_from_geo_info` stay readable and so a future
+    diff touching only the geographic tier does not need to repeat the
+    ``reason=`` clause at every call site.
+    """
+    _emit_deprecated_geokey_attr(
+        attrs, name, value,
+        reason=_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS,
+    )
+
+
 def _extent_to_window(transform, file_height, file_width,
                       y_min, y_max, x_min, x_max):
     """Convert geographic extent to pixel window (row_start, col_start, row_stop, col_stop).
@@ -254,67 +447,47 @@ def _populate_attrs_from_geo_info(attrs: dict, geo_info, *, window=None) -> None
         )
 
     if geo_info.crs_name is not None:
-        attrs['crs_name'] = geo_info.crs_name
+        _emit_deprecated_geographic_geokey(attrs, 'crs_name', geo_info.crs_name)
     if geo_info.geog_citation is not None:
-        attrs['geog_citation'] = geo_info.geog_citation
+        _emit_deprecated_geographic_geokey(
+            attrs, 'geog_citation', geo_info.geog_citation)
     if geo_info.datum_code is not None:
-        attrs['datum_code'] = geo_info.datum_code
+        _emit_deprecated_geographic_geokey(
+            attrs, 'datum_code', geo_info.datum_code)
     if geo_info.angular_units is not None:
-        attrs['angular_units'] = geo_info.angular_units
+        _emit_deprecated_geographic_geokey(
+            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,
+        _emit_deprecated_geokey_attr(
+            attrs, 'linear_units', geo_info.linear_units,
+            reason=_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS,
         )
-        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
+        _emit_deprecated_geographic_geokey(
+            attrs, 'semi_major_axis', geo_info.semi_major_axis)
     if geo_info.inv_flattening is not None:
-        attrs['inv_flattening'] = geo_info.inv_flattening
+        _emit_deprecated_geographic_geokey(
+            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,
+        _emit_deprecated_geokey_attr(
+            attrs, 'projection_code', geo_info.projection_code,
+            reason=_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS,
         )
-        attrs['projection_code'] = geo_info.projection_code
     if geo_info.vertical_epsg is not None:
-        warnings.warn(
-            "xrspatial.geotiff: attrs['vertical_crs'] is deprecated; "
-            "the writer cannot reconstruct vertical-CRS GeoKeys so it "
-            "will not round-trip. It will be removed in a future "
-            "release. See issue #1984.",
-            DeprecationWarning,
-            stacklevel=2,
+        _emit_deprecated_geokey_attr(
+            attrs, 'vertical_crs', geo_info.vertical_epsg,
+            reason=_GEOKEY_DEPRECATION_REASON_VERTICAL_CRS,
         )
-        attrs['vertical_crs'] = geo_info.vertical_epsg
     if geo_info.vertical_citation is not None:
-        warnings.warn(
-            "xrspatial.geotiff: attrs['vertical_citation'] is deprecated; "
-            "the writer cannot reconstruct vertical-CRS GeoKeys so it "
-            "will not round-trip. It will be removed in a future "
-            "release. See issue #1984.",
-            DeprecationWarning,
-            stacklevel=2,
+        _emit_deprecated_geokey_attr(
+            attrs, 'vertical_citation', geo_info.vertical_citation,
+            reason=_GEOKEY_DEPRECATION_REASON_VERTICAL_CRS,
         )
-        attrs['vertical_citation'] = geo_info.vertical_citation
     if geo_info.vertical_units is not None:
-        warnings.warn(
-            "xrspatial.geotiff: attrs['vertical_units'] is deprecated; "
-            "the writer cannot reconstruct vertical-CRS GeoKeys so it "
-            "will not round-trip. It will be removed in a future "
-            "release. See issue #1984.",
-            DeprecationWarning,
-            stacklevel=2,
+        _emit_deprecated_geokey_attr(
+            attrs, 'vertical_units', geo_info.vertical_units,
+            reason=_GEOKEY_DEPRECATION_REASON_VERTICAL_CRS,
         )
-        attrs['vertical_units'] = geo_info.vertical_units
 
     if geo_info.gdal_metadata is not None:
         attrs['gdal_metadata'] = geo_info.gdal_metadata