Skip to content

geotiff: add attrs canonical-tier locking test (#1984)#2009

Merged
brendancol merged 2 commits into
mainfrom
1984-pr4-attrs-canonical-locking-test
May 18, 2026
Merged

geotiff: add attrs canonical-tier locking test (#1984)#2009
brendancol merged 2 commits into
mainfrom
1984-pr4-attrs-canonical-locking-test

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

Coverage

test_attrs_contract_canonical_1984.py builds one synthetic DataArray that sets every canonical attr, round-trips it through to_geotiff -> open_geotiff, and runs per-key assertions:

Key Assertion
crs EPSG int round-trips
crs_wkt reader-emitted WKT contains the CRS identity
transform exact 6-tuple round-trip (pixel-centre coords -> corner origin)
nodata declared sentinel survives
extra_tags unknown TIFF tag (Software, 305 ASCII) preserved verbatim
gdal_metadata parsed dict survives key-by-key
gdal_metadata_xml raw XML reconstructed by the writer
x_resolution / y_resolution / resolution_unit resolution group survives as a unit
_xrspatial_geotiff_contract stamped on the round-tripped DataArray
raster_type 'point' round-trips; absence (= implicit 'area') is preserved

A separate test_every_canonical_key_present runs the same fixture and asserts the full canonical set is present, so a writer that drops one key surfaces as one missing key rather than as a cascade of equality failures.

Why this shape

The 7-PR plan called for "a single fixture exercising every attr, with explicit assertions per tier." Per-key tests share one fixture so a single canonical-attr regression points at the right key in the failure message. Issues #1985 (parity matrix) and #1986 (round-trip invariants) will import this assertion list.

Test plan

  • pytest xrspatial/geotiff/tests/test_attrs_contract_canonical_1984.py -- 12 passed
  • pytest xrspatial/geotiff/tests/test_attrs_contract_aliases_1984.py xrspatial/geotiff/tests/test_attrs_contract_passthrough_1984.py xrspatial/geotiff/tests/test_attrs_contract_version_1984.py -- 31 passed (sibling tests unaffected)

PR 4 of 7 from the attrs-contract plan in #1984. Adds
test_attrs_contract_canonical_1984.py: a single fixture that exercises
every canonical attr (crs, crs_wkt, transform, nodata, raster_type,
extra_tags, gdal_metadata, gdal_metadata_xml, x_resolution / y_resolution /
resolution_unit, _xrspatial_geotiff_contract), round-trips it through
to_geotiff -> open_geotiff, and asserts presence + value per key.

Sibling locking tests already cover the other tiers: aliases (#2002),
pass-through (#2004), and per-backend stamping of the contract version
(#2003). The canonical assertion list is what #1985 (parity matrix) and
#1986 (round-trip invariants) will import.

No production-code changes.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 17, 2026
Addresses self-review on PR #2009:

* Add per-backend canonical-key presence parametrisation
  (eager-numpy, dask-numpy, gpu, dask-gpu). The 7-PR plan in #1984
  called for "one fixture per backend" coverage; previously only the
  eager read path was exercised.
* Comment ``_CANONICAL_KEYS`` to explain the deliberate omission of
  ``raster_type`` (the implicit 'area' default is encoded as absence,
  so the constant cannot express it; the two dedicated raster_type
  tests handle both branches).
* Drop ``AREA_OR_POINT: 'Area'`` from the shared ``_GDAL_META``
  fixture so the point-branch test does not inherit an inconsistent
  GDAL_METADATA entry.
* Relax the crs_wkt substring check to a regex covering 'WGS 84',
  'WGS_1984', and 'WGS-84' so the assertion survives PROJ-version
  variation across CI platforms.
* Note in a comment that ``gdal_metadata['TIFFTAG_SOFTWARE']`` and the
  raw Software TIFF tag (305) in ``extra_tags`` are independent
  channels the writer does not synchronise.
@brendancol brendancol merged commit 198a397 into main May 18, 2026
5 checks passed
brendancol added a commit that referenced this pull request May 18, 2026
… (#2010)

The writer in xrspatial.geotiff._geotags.build_geo_tags never emits the
vertical GeoKey block, so attrs['vertical_crs'], attrs['vertical_citation']
and attrs['vertical_units'] cannot round-trip. PR6 (#2009) locked this
"dropped on round-trip" behaviour. PR7 deprecates the three read-side
emissions with a DeprecationWarning for one release cycle before removal.

The attrs still emit; only the warning is new.
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