reproject: align attrs['vertical_crs'] with geotiff (EPSG int)#1574
Merged
brendancol merged 3 commits intoMay 11, 2026
Conversation
xrspatial.geotiff.open_geotiff() writes attrs['vertical_crs'] as an EPSG
integer (e.g. 5773 for EGM96), while reproject() was writing it as a
string token ('EGM96', 'EGM2008', 'ellipsoidal'). Same key, incompatible
types -- breaks cross-module round trips and downstream code that
inspects the attr.
reproject() now writes the EPSG integer code and preserves the friendly
token under attrs['vertical_datum']. The src_vertical_crs / tgt_vertical_crs
kwargs still accept the string tokens, so existing call sites do not
need to change.
Behavior change: attrs['vertical_crs'] on reproject() output is now an
int rather than a string. Searched the repo; no notebooks or other
modules consume this attr.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses API consistency between xrspatial.reproject.reproject() and xrspatial.geotiff.open_geotiff() by making reproject() write attrs['vertical_crs'] as an EPSG integer code (matching the GeoTIFF reader) instead of a friendly string token, while preserving the token under a new attrs['vertical_datum'].
Changes:
- Introduced a token→EPSG mapping for vertical datums and updated
reproject()to emit EPSG ints inattrs['vertical_crs'], plusattrs['vertical_datum']for the original token. - Updated and added tests to assert the new attribute semantics for EGM96 / EGM2008 / ellipsoidal cases.
- Logged the sweep state update for the API consistency pass.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xrspatial/reproject/__init__.py |
Emit EPSG integer vertical CRS codes (and preserve friendly token under vertical_datum) to align with GeoTIFF conventions. |
xrspatial/tests/test_reproject.py |
Update existing assertion and add coverage ensuring vertical_crs is an EPSG int and vertical_datum preserves the token. |
.claude/sweep-api-consistency-state.csv |
Record inspection state and notes for the reproject module consistency sweep. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR #1574 review: - reproject() now raises ValueError if src_vertical_crs or tgt_vertical_crs is set to a token outside _VERTICAL_DATUM_EPSG. The previous .get() silently wrote None to attrs['vertical_crs'] for typos / unsupported values, producing outputs with vertical_datum set but vertical_crs missing. - Update the docstring's Returns section to say the vertical attrs are written whenever tgt_vertical_crs is set (not only "if a vertical transformation was applied"), matching the actual behaviour when src == tgt or only the target is provided. - Add test_unknown_vertical_crs_raises covering both kwargs.
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
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
reproject()was writingattrs['vertical_crs']as a string token ('EGM96','EGM2008','ellipsoidal'), whilexrspatial.geotiff.open_geotiff()writes the same attr as an EPSG integer (e.g.5773). Same key, incompatible types across two public read/write paths.reproject()now writes the EPSG integer code (5773 / 3855 / 4979) toattrs['vertical_crs']to match the geotiff convention. The friendly string token is preserved underattrs['vertical_datum']so the human-readable name is not lost.src_vertical_crs/tgt_vertical_crskwargs still accept the string tokens, so call sites do not change.Behavior change
The type of
attrs['vertical_crs']onreproject()output changes fromstrtoint. Searched the repo: no notebooks, tests, or other modules read this attr off a reproject result, so external blast radius is minimal. The one existing test that asserted the string form was updated.Test plan
TestVerticalShift::test_reproject_egm96_to_ellipsoidalto assert the new EPSG int formTestVerticalShift::test_vertical_crs_attr_is_epsg_intcovering EGM96 / EGM2008 / ellipsoidal targetspytest xrspatial/tests/test_reproject.py-- 194 pass