to_geotiff: round-trip descending x and ascending y (#1716)#1730
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes GeoTIFF round-tripping for DataArrays with non-standard axis orientation (descending x and/or ascending y) by encoding the full signed affine transform in GeoTIFF tags, and updates VRT mosaic placement to correctly compute offsets for these orientations.
Changes:
- Write
ModelTransformationTag(34264) instead ofModelPixelScaleTag+ModelTiepointTagwhen the transform has descendingxor ascendingy. - Teach the writer to serialize
ModelTransformationTag(16 doubles) into IFDs for both regular and streaming writes. - Fix
write_vrtdestination offsets to use each source’s true top/left edges (works for ascending-ysources) and add regression tests for #1716.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xrspatial/geotiff/_geotags.py |
Emits ModelTransformationTag for non-north-up transforms to preserve pixel scale sign. |
xrspatial/geotiff/_writer.py |
Serializes ModelTransformationTag values into TIFF tags (16 doubles). |
xrspatial/geotiff/_vrt.py |
Computes VRT tile offsets from source top/left extent rather than assuming north-up origin. |
xrspatial/geotiff/tests/test_descending_coords_1716.py |
Adds regression coverage for descending/ascending coordinate round-trips and tag layout expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11
to
+14
| import os | ||
|
|
||
| import numpy as np | ||
| import pytest |
Comment on lines
+912
to
+916
| # maps (col, row, 0, 1) -> (X, Y, 0, 1). | ||
| tags[TAG_MODEL_TRANSFORMATION] = ( | ||
| transform.pixel_width, 0.0, 0.0, transform.origin_x, | ||
| 0.0, transform.pixel_height, 0.0, transform.origin_y, | ||
| 0.0, 0.0, 0.0, 0.0, |
The writer stored abs(pixel_width)/abs(pixel_height) in ModelPixelScaleTag, and the reader hard-coded a north-up reconstruction. DataArrays with descending x or ascending y silently round-tripped with the wrong sign. Emit ModelTransformationTag (34264) for non-standard orientations: that tag carries the full signed 4x4 affine, which ModelPixelScale cannot because the spec requires positive scales. The reader already preferred ModelTransformationTag when present; only the writer side needed work. Also fix write_vrt's dst_y_off / dst_x_off math to use the top-left corner of each source's geographic extent rather than blindly assuming origin_y is the top. Without this, mosaics built from sources with ascending y placed tiles outside the VRT extent.
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
to_geotifflost the sign ofpixel_widthandpixel_heightwhen writing DataArrays with descending x or ascending y coords. The writer storedabs(...)in ModelPixelScaleTag (33550) and the reader hard-coded a north-up reconstruction, so the round trip silently changed the georeference.Repro
Fix
build_geo_tags): when orientation is non-standard (pixel_width < 0orpixel_height > 0), emit ModelTransformationTag (34264) instead of ModelPixelScaleTag + ModelTiepointTag. ModelPixelScale requires positive scales per the GeoTIFF spec, so the transformation tag's signed 4x4 affine is the right route here.write_vrt: also fixeddst_y_off/dst_x_offto use the top-left corner of each source's geographic extent rather thanorigin_y/origin_xdirectly. Without this, mosaics built from sources with ascending y placed tiles outside the VRT extent (which the new writer behavior now exposes for the first time).Tests
New
xrspatial/geotiff/tests/test_descending_coords_1716.py:test_descending_x_roundtrip-- descending x coords survivetest_ascending_y_roundtrip-- ascending y coords survivetest_descending_x_and_ascending_y_roundtrip-- both axes flippedtest_north_up_still_uses_pixel_scale_and_tiepoint-- standard orientation keeps the existing tag layouttest_descending_x_uses_transformation_tag-- non-standard orientation emits 34264 and omits 33550 / 33922test_ascending_y_uses_transformation_tag-- same check on the y axisFull geotiff test suite passes apart from two pre-existing failures (
test_window_clamps_to_raster_bounds,test_window_clamps_negative_offsets) that fail onmainand are unrelated.Closes #1716