geotiff: deprecate matplotlib colormap variants in DataArray.attrs (#1984)#2014
Merged
Conversation
…1984) Issue #1984 PR 7. The reader emits ``attrs['cmap']`` (when matplotlib is importable) and ``attrs['colormap_rgba']`` whenever the source file declares Photometric=3 (palette). The writer never selects Photometric=3 from attrs alone, so PR 6 (#2004) locked the round-trip drop for both keys as part of the attrs-contract pass-through tier. This PR deprecates the read-side emission of both keys for one release cycle. Callers who want a matplotlib ListedColormap should build one from the canonical ``attrs['colormap']`` (raw uint16 RGB triples from TIFF tag 320) instead. ``attrs['colormap']`` is the canonical-tier replacement and is intentionally kept: it round-trips through ``_merge_friendly_extra_tags`` already. During the deprecation window both attrs still land on the returned DataArray; only a DeprecationWarning is added. Removal is a follow-up PR. The new attrs-contract docstring split moves both keys into a "Deprecated" section so the contract stays explicit about which keys have a future. The new test file ``xrspatial/geotiff/tests/test_attrs_pr7_deprecate_colormap_variants_1984.py`` pins three behaviours: the warning fires on a Photometric=3 fixture for ``colormap_rgba`` (always) and for ``cmap`` (when matplotlib is installed); the attrs are still emitted; the plain ``attrs['colormap']`` key is unaffected. Existing tests that read palette fixtures (the TestPalette block in ``test_features.py`` and ``test_colormap_round_trip`` in ``test_metadata_round_trip_1484.py``) ignore the new DeprecationWarning locally to keep their reports clean.
PR review on #2014 flagged that the three colormap deprecation sites copy-paste the same nine-line ``warnings.warn(...)`` block with only the attr name and migration hint varying. Sibling PRs #2010 / #2013 have the same shape; PR #2011 already factors a slice-specific helper. Introduce a generic helper in ``_attrs.py`` so all four PR 7 slices can share it. ``_emit_deprecated_geokey_attr(attrs, name, value, *, reason, migration=None)`` builds the canonical warning text shape: xrspatial.geotiff: attrs['<name>'] is deprecated; <reason>. [<migration>.] It will be removed in a future release. See issue #1984. then sets ``attrs[name] = value`` (read-side emission still alive for the deprecation window). ``stacklevel=2`` lands the warning at the caller of ``_populate_attrs_from_geo_info``; ``DeprecationWarning`` is silenced for library code by default in Python's filters, so this is mainly visible to test runners and developers who opt in. The colormap-variants deprecation now routes ``cmap`` and the two ``colormap_rgba`` branches (matplotlib present + ImportError fallback) through the helper, replacing 27 lines of copy-paste with three calls. The emitted warning text is byte-identical to the previous inline strings, so the existing tests (``pytest.warns`` plus the TestPalette / test_metadata_round_trip filter regexes) keep passing unchanged. Sibling PRs #2010 and #2013 can adopt the helper on rebase; #2011's existing geographic-specific helper can be inlined onto this one in a follow-up. Test plan: - pytest xrspatial/geotiff/tests/test_attrs_pr7_deprecate_colormap_variants_1984.py -- 4 passed. - pytest xrspatial/geotiff/tests/test_attrs_contract_*_1984.py xrspatial/geotiff/tests/test_metadata_round_trip_1484.py -- 57 passed. - pytest xrspatial/geotiff/tests/test_features.py::TestPalette -- 7 passed.
Contributor
Author
PR Review: geotiff: deprecate matplotlib colormap variants in DataArray.attrsReview covers both commits on the branch: the original colormap deprecation ( Blockers (must fix before merge)
Suggestions (should fix, not blocking)
Nits (optional improvements)
What looks good
Checklist
|
Self-review on PR #2014 flagged one blocker and two suggestions on the shared-helper refactor. This commit addresses them. Blocker -- ``stacklevel`` regression (``_attrs.py:179``): Wrapping the inline ``warnings.warn(..., stacklevel=2)`` call in a helper shifted the warning location by one frame; the warning was surfacing at ``_attrs.py`` (the helper call site) instead of at the backend that called ``_populate_attrs_from_geo_info``. Bumped ``stacklevel=2`` to ``stacklevel=3`` so the warning is attributed to the backend frame, matching the pre-refactor behaviour (verified empirically: warning now surfaces at ``xrspatial/geotiff/__init__.py:518`` instead of ``_attrs.py:350``). Suggestion -- helper name (``_attrs.py:152``): Renamed ``_emit_deprecated_geokey_attr`` to ``_emit_deprecated_attr`` because the colormap variants (cmap, colormap_rgba) come from TIFF tag 320 (ColorMap), not from a GeoKey. The new name covers every PR 7 slice; docstring notes why the ``_geokey_`` qualifier was dropped. Suggestion -- user-guide doc: Added a new "Deprecated keys" section to ``docs/source/user_guide/attrs_contract.rst`` so users reading the contract page see the new tier alongside the existing canonical / compatibility-alias / pass-through tiers. Mirrors the module docstring. Suggestion -- direct unit test for the helper: Added ``test_emit_deprecated_attr_with_migration_text`` and ``test_emit_deprecated_attr_without_migration_text``. The first pins the exact warning text shape; the second pins the ``migration=None`` branch which the existing colormap tests do not exercise. Nit -- migration hints: Lifted ``_colormap_reason`` / ``_colormap_migration`` to module- level constants and split them into ``_DEPRECATED_COLORMAP_REASON``, ``_DEPRECATED_CMAP_MIGRATION``, and ``_DEPRECATED_COLORMAP_RGBA_MIGRATION``. The new ``colormap_rgba``-specific migration ("Reshape attrs['colormap'] to (n_colors, 3) and append an alpha channel") is more direct than the previous shared ``ListedColormap`` hint, which only matched ``cmap`` cleanly. Test plan: - pytest test_attrs_pr7_deprecate_colormap_variants_1984.py (6 passed) - pytest test_attrs_contract_*_1984.py test_metadata_round_trip_1484.py test_features.py::TestPalette -- 70 passed total
Two textual conflicts in files where the colormap-variants slice and the sibling PR 7 slices (#2010 vertical, #2013 projected, #2012 doc nits) both edited the same regions: xrspatial/geotiff/_attrs.py (module docstring) Origin/main moved linear_units / projection_code / vertical_crs / vertical_citation / vertical_units from the pass-through section into a new "Deprecated" section, and kept the historical combined ``colormap, colormap_rgba, cmap`` line in pass-through. HEAD split that line: ``colormap`` stays in pass-through (with detail about the uint16 RGB triples it carries), ``colormap_rgba`` and ``cmap`` move to the "Deprecated" section. Resolution: keep main's five deprecated entries and add HEAD's two below them, so all seven PR 7 deprecations appear in one section in the order the slices were authored. docs/source/user_guide/attrs_contract.rst Origin/main (via #2012) tightened the colormap_rgba and cmap row text in the pass-through table but left both keys in the pass- through tier. HEAD moved both rows into a new "Deprecated keys" section with the deprecation rationale and a per-row migration recipe. Resolution: keep HEAD's "Deprecated keys" section as the new home for both keys; the pass-through table no longer lists them (already resolved by the auto-merge on adjacent lines). Pre-existing inconsistency not addressed here: origin/main now marks ``linear_units``, ``projection_code``, and the vertical-CRS attrs as deprecated in the Python module docstring but still lists them under "Pass-through keys" in the RST user-guide page. That gap was introduced by #2010 / #2013, which did not update the RST. Fixing it is scope creep for this PR; flag for the next #1984 follow-up. Test plan: - pytest test_attrs_pr7_deprecate_colormap_variants_1984.py (6 passed) - pytest test_attrs_pr7_deprecate_vertical_1984.py (4 passed) - pytest test_attrs_pr7_deprecate_projected_1984.py (3 passed) - pytest test_attrs_contract_*_1984.py test_metadata_round_trip_1484.py test_features.py::TestPalette -- 77 passed total, no regressions.
PR #2011 (geographic-CRS GeoKey deprecation) merged to main after my first merge commit (ac908ab) and reshaped both the module docstring and the helper layout. This commit reconciles the colormap-variants slice with that landing. xrspatial/geotiff/_attrs.py (helper layout) Origin/main now ships: * ``_DEPRECATED_GEOGRAPHIC_GEOKEY_ATTRS`` tuple of attr names. * Two per-category reason constants (``_GEOKEY_DEPRECATION_REASON_HORIZONTAL_CRS`` and ``_GEOKEY_DEPRECATION_REASON_VERTICAL_CRS``). * ``_deprecated_geokey_warning(name, *, reason)`` text builder with the canonical wording "...{reason} so it will not round-trip..." pinned by sibling tests. * ``_deprecated_geographic_geokey_warning(name)`` backward-compat shim that fixes the reason to the horizontal-CRS clause. * ``_stacklevel_to_external_caller()`` stack walker that walks past every ``xrspatial.geotiff*`` frame so deprecation warnings point at the user's ``open_geotiff(...)`` call rather than at internal read paths. Materially better than a fixed ``stacklevel=`` constant because the chain depth differs by backend (numpy: 3 frames; dask: 5). * ``_emit_deprecated_geokey_attr(attrs, name, value, *, reason)`` emitter that pairs the text builder with the stack walker. * ``_emit_deprecated_geographic_geokey(attrs, name, value)`` geographic-tier shim. HEAD had ``_DEPRECATED_COLORMAP_REASON`` / ``_DEPRECATED_CMAP_MIGRATION`` / ``_DEPRECATED_COLORMAP_RGBA_MIGRATION`` constants plus an ``_emit_deprecated_attr(attrs, name, value, *, reason, migration=None)`` emitter with a fixed ``stacklevel=3``. The colormap helper needed migration-text support (e.g. "Reshape attrs['colormap'] to (n_colors, 3) ...") that the GeoKey-tier fixed-template helper does not support. Resolution: kept both helpers, switched the colormap helper from ``stacklevel=3`` to ``_stacklevel_to_external_caller()`` so all four PR 7 slices attribute warnings consistently. The two helpers live side-by-side because their warning-text contracts are pinned by separate test suites; convergence is left as a follow-up. xrspatial/geotiff/_attrs.py (module docstring "Deprecated" section) Origin/main restructured the section into three per-axis subsections (Geographic-CRS / Projected-CRS / Vertical-CRS) plus a pyproj migration recipe. Added a fourth subsection "Colormap variants (different root cause: photometric gate, not GeoKey)" with the two colormap-derived entries. Test plan: - pytest xrspatial/geotiff/tests/test_attrs_pr7_deprecate_*_1984.py (colormap + vertical + projected + geographic) -- 23 passed. - pytest xrspatial/geotiff/tests/test_attrs_contract_*_1984.py xrspatial/geotiff/tests/test_metadata_round_trip_1484.py xrspatial/geotiff/tests/test_features.py::TestPalette -- 94 passed total, no regressions.
This was referenced May 18, 2026
brendancol
added a commit
that referenced
this pull request
May 18, 2026
…2015) The four PR 7 deprecation slices (#2010 vertical, #2011 geographic, #2013 projected, #2014 colormap variants) moved 13 attrs from the pass-through tier to a new deprecated tier in ``xrspatial/geotiff/_attrs.py`` but did not update the user-guide page, which left ``docs/source/user_guide/attrs_contract.rst`` listing those attrs as round-trip-best-effort while the Python code fires a DeprecationWarning every time one is emitted. This is docs-only: * Trim ``Pass-through keys`` table to ``image_description``, ``extra_samples``, ``colormap`` -- the three attrs that genuinely round-trip via ``_merge_friendly_extra_tags``. The 11 GeoKey- derived attrs plus the two colormap variants moved out. * Merge the two separate deprecated sections that existed pre-PR (``Deprecated keys`` for colormap variants, ``Deprecated GeoKey attrs`` for the GeoKey slices) into one ``Deprecated keys`` section with two subsections: GeoKey-derived attrs (with the shared pyproj migration recipe) and matplotlib colormap variants (with per-row migration in a list-table). * Update the page's opening summary from "three tiers" to "four tiers" so the first paragraph matches the table-of-contents. * Update the ``Round-trip invariants`` section: the pass-through subsection now reflects that only three attrs live there and all three round-trip; add a deprecated subsection that explains the warning-only phase and points at the future contract-version bump to 2 when removal lands. No production-code change. Verified the file still parses with docutils (Sphinx-specific ``:func:`` / ``:mod:`` role warnings are the same pattern used across the rest of the docs). Refs #1984.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #1984 set up a three-tier attrs contract for the geotiff reader/writer pair. PR 6 (#2004) locked which pass-through attrs round-trip through
open_geotiff -> to_geotiff -> open_geotiff. Two keys in that tier do NOT round-trip:attrs['cmap'](matplotlibListedColormap) andattrs['colormap_rgba'](RGBA palette array). The reader only emits both when the source file declaresPhotometric=3(palette images), and the writer never selectsPhotometric=3from attrs alone, so the two keys disappear on any write-then-read cycle.This PR deprecates read-side emission of both keys for one release cycle. Callers that want a
ListedColormapshould build one from the canonicalattrs['colormap'](raw uint16 RGB triples from TIFF tag 320) instead; that key stays in the pass-through tier because the writer folds it intoextra_tagsvia_merge_friendly_extra_tagsand the reader rebuilds the attr from tag 320 on the next read.Scope of the change:
xrspatial/geotiff/_attrs.py: wrap each of the two assignments in_populate_attrs_from_geo_infowithwarnings.warn(..., DeprecationWarning, stacklevel=2). Both attrs continue to be emitted on the returned DataArray during the deprecation window; only the warning is new. The matplotlib import is conditional, so thecolormap_rgbawarning fires on both branches (matplotlib present andImportErrorfallback).colormap, colormap_rgba, cmapline.colormapstays in "Best-effort pass-through".colormap_rgbaandcmapmove to a new "Deprecated (will be removed in a future release; see issue geotiff: define a public contract for DataArray attrs (canonical / alias / pass-through) #1984)" section.CHANGELOG.md: one line under Unreleased.xrspatial/geotiff/tests/test_attrs_pr7_deprecate_colormap_variants_1984.py(4 tests) pinning: warning fires forcolormap_rgba(always) andcmap(matplotlib-only); both attrs still emit during the window; plainattrs['colormap']is unaffected.test_features.py::TestPaletteandtest_metadata_round_trip_1484.py::test_colormap_round_tripignore the newDeprecationWarninglocally to keep their reports clean.Warning text (both keys;
<name>iscmaporcolormap_rgba):Related: #1984, #2004.
Test plan
pytest xrspatial/geotiff/tests/test_attrs_pr7_deprecate_colormap_variants_1984.py-- 4 passed.pytest xrspatial/geotiff/tests/test_attrs_contract_{passthrough,canonical,aliases,version}_1984.py-- 47 passed.pytest xrspatial/geotiff/tests/test_metadata_round_trip_1484.py-- 10 passed, no new warning leaks.pytest xrspatial/geotiff/tests/test_features.py::TestPalette-- 7 passed, no new warning leaks.