Commit d1bd260
authored
* geotiff: deprecate matplotlib colormap variants in DataArray.attrs (#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.
* geotiff: factor shared _emit_deprecated_geokey_attr helper
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.
* geotiff: address self-review on shared deprecation helper
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
1 parent 20751b5 commit d1bd260
6 files changed
Lines changed: 483 additions & 12 deletions
File tree
- docs/source/user_guide
- xrspatial/geotiff
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
180 | 180 | | |
181 | 181 | | |
182 | 182 | | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
183 | 201 | | |
184 | 202 | | |
185 | | - | |
186 | | - | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
187 | 209 | | |
188 | 210 | | |
189 | | - | |
190 | | - | |
191 | | - | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
192 | 215 | | |
193 | 216 | | |
194 | 217 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
67 | | - | |
| 66 | + | |
| 67 | + | |
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
106 | 117 | | |
107 | 118 | | |
108 | 119 | | |
| |||
208 | 219 | | |
209 | 220 | | |
210 | 221 | | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
211 | 241 | | |
212 | 242 | | |
213 | 243 | | |
| |||
332 | 362 | | |
333 | 363 | | |
334 | 364 | | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
335 | 413 | | |
336 | 414 | | |
337 | 415 | | |
| |||
513 | 591 | | |
514 | 592 | | |
515 | 593 | | |
516 | | - | |
517 | | - | |
518 | | - | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
519 | 605 | | |
520 | | - | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
521 | 611 | | |
522 | 612 | | |
523 | 613 | | |
| |||
0 commit comments