Skip to content

geotiff: add attrs contract documentation page (#1984)#2001

Merged
brendancol merged 1 commit into
mainfrom
1984-pr2-attrs-contract-rst
May 17, 2026
Merged

geotiff: add attrs contract documentation page (#1984)#2001
brendancol merged 1 commit into
mainfrom
1984-pr2-attrs-contract-rst

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

PR 2 of 7 implementing issue #1984.

Summary

Adds a user-guide page documenting the three-tier attrs contract emitted by the geotiff readers and consumed by the writers.

  • Canonical keys (xrspatial-owned, byte-stable round-trip): crs, crs_wkt, transform, nodata, raster_type, extra_tags, gdal_metadata, gdal_metadata_xml, x_resolution, y_resolution, resolution_unit, _xrspatial_geotiff_contract.
  • Compatibility aliases (read-only, never re-emitted): nodatavals (rioxarray), _FillValue (CF).
  • Pass-through (GeoKey-derived; writer reconstructs what it can from CRS, drops the rest).

The page states per-tier round-trip invariants and explains the _xrspatial_geotiff_contract = 1 version key for future revisions.

Test plan

  • reST parses cleanly under docutils (Sphinx-specific :func: roles flagged as unknown, same pattern as the existing docs/source/reference/geotiff.rst).
  • Sphinx build in CI confirms the new page renders and shows up in the user-guide toctree (local sphinx-build fails on an unrelated conf.py import error from a cupy/dask/python 3.14 mismatch; CI runs in a clean env).
  • docs/source/user_guide/index.rst lists attrs_contract in the toctree so the page is reachable.

Adds docs/source/user_guide/attrs_contract.rst describing the three-tier
attrs contract enforced by the geotiff entry points:

* Canonical keys owned by xrspatial (crs, crs_wkt, transform, nodata,
  raster_type, extra_tags, gdal_metadata, gdal_metadata_xml,
  x_resolution, y_resolution, resolution_unit,
  _xrspatial_geotiff_contract).
* Compatibility aliases recognised on read but never re-emitted on
  write (nodatavals, _FillValue).
* Pass-through keys surfaced from GeoKeys; the writer reconstructs
  what it can from crs / crs_wkt and drops the rest silently.

Documents per-tier round-trip invariants and the contract version key
(_xrspatial_geotiff_contract = 1). Links the new page from
docs/source/user_guide/index.rst.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 17, 2026
@brendancol
Copy link
Copy Markdown
Contributor Author

PR Review: geotiff: add attrs contract documentation page

Blockers (must fix before merge)

  • docs/source/user_guide/attrs_contract.rst:54-58 — the transform description is wrong. The doc says (origin_x, pixel_width, 0, origin_y, 0, pixel_height) affine transform tuple matching the GDAL ordering. The emitted tuple is rasterio-style: (pixel_width, 0.0, origin_x, 0.0, pixel_height, origin_y) (see xrspatial/geotiff/_coords.py:91-115). Users who follow the doc to construct or interpret attrs['transform'] will compute wrong georeferenced coordinates. Fix the tuple layout and replace "GDAL ordering" with "rasterio Affine ordering".

Suggestions (should fix, not blocking)

  • docs/source/user_guide/attrs_contract.rst:51crs_wkt is described as a "WKT2 string". Confirm the reader emits WKT2 (not WKT1) for every CRS source; if it stores whatever the file carries, drop the specific dialect.
  • docs/source/user_guide/attrs_contract.rst:185cmap is keyed off matplotlib availability and the doc says "Present only when matplotlib is importable." colormap_rgba is also conditional on the file's photometric interpretation (Photometric == 3); worth noting in the same row.

Nits (optional improvements)

  • docs/source/user_guide/attrs_contract.rst:60-64 — the nodata row says "consumed by writers as the primary nodata source." The precedence chain on read is nodata, then nodatavals, then _FillValue, codified in _resolve_nodata_attr. Worth a one-liner.

What looks good

  • Three-tier table layout reads as a reference.
  • Round-trip invariants section spells out each tier's guarantee clearly.
  • Toctree entry added so the page is reachable from the user-guide index.

Checklist

@brendancol brendancol merged commit 79a0a78 into main May 17, 2026
6 of 7 checks passed
brendancol added a commit that referenced this pull request May 17, 2026
- transform: corrected layout to the rasterio Affine ordering
  (pixel_width, 0.0, origin_x, 0.0, pixel_height, origin_y) that
  _transform_tuple_from_pixel_geometry actually emits, and replaced
  the "GDAL ordering" label with "rasterio Affine ordering".
- crs_wkt: relaxed "Always present" and "WKT2 string" to reflect that
  the dialect depends on the source path (pyproj synthesis is WKT2;
  VRT SRS-tag passthrough carries whatever dialect was on disk).
- nodata: documented the read-side precedence chain (nodata,
  nodatavals, _FillValue) as codified in _resolve_nodata_attr.
- colormap_rgba and cmap: noted the Photometric == 3 gate that the
  reader applies before emitting either attr.
brendancol added a commit that referenced this pull request May 18, 2026
Two nits from the post-merge reviews on #2001 and #2002 that did not
make it into the merged commits.

#2001 user-guide page (``docs/source/user_guide/attrs_contract.rst``):
- ``transform``: corrected layout from
  ``(origin_x, pixel_width, 0, origin_y, 0, pixel_height)`` (GDAL
  ordering) to the rasterio Affine ordering
  ``(pixel_width, 0.0, origin_x, 0.0, pixel_height, origin_y)`` that
  ``_transform_tuple_from_pixel_geometry`` actually emits.
- ``crs_wkt``: relaxed "Always present" and "WKT2 string" to reflect
  that the dialect depends on the source path (pyproj synthesis is
  WKT2; VRT SRS-tag passthrough carries whatever dialect was on disk).
- ``nodata``: documented the read-side precedence chain (``nodata``,
  then ``nodatavals``, then ``_FillValue``) as codified in
  ``_resolve_nodata_attr``.
- ``colormap_rgba`` / ``cmap``: noted the ``Photometric == 3`` gate
  that the reader applies before emitting either attr.

#2002 alias locking test
(``xrspatial/geotiff/tests/test_attrs_contract_aliases_1984.py``):
- ``_SENTINEL``: added a one-line comment that the value just needs to
  be float-castable and distinct from data; the resolver and
  round-trip assertions are what the tests pin.

Coordinates with the canonical-tier docstring already corrected on
``_attrs.py`` in main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant