Skip to content

to_geotiff: round-trip descending x and ascending y (#1716)#1730

Merged
brendancol merged 1 commit into
mainfrom
issue-1716
May 12, 2026
Merged

to_geotiff: round-trip descending x and ascending y (#1716)#1730
brendancol merged 1 commit into
mainfrom
issue-1716

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

to_geotiff lost the sign of pixel_width and pixel_height when writing DataArrays with descending x or ascending y coords. The writer stored abs(...) in ModelPixelScaleTag (33550) and the reader hard-coded a north-up reconstruction, so the round trip silently changed the georeference.

Repro

import numpy as np
import xarray as xr
from xrspatial.geotiff import to_geotiff, open_geotiff

x = np.array([200.0, 190.0, 180.0, 170.0, 160.0])  # descending
y = np.array([50.0, 40.0, 30.0, 20.0])
arr = np.arange(20, dtype=np.float32).reshape(4, 5)
da = xr.DataArray(arr, dims=('y', 'x'), coords={'y': y, 'x': x})
to_geotiff(da, '/tmp/out.tif', crs=4326)
print(open_geotiff('/tmp/out.tif').coords['x'].values)
# Before: [-200. -190. -180. -170. -160.]  (sign flipped)
# After:  [ 200.  190.  180.  170.  160.]

Fix

  • Writer (build_geo_tags): when orientation is non-standard (pixel_width < 0 or pixel_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.
  • North-up branch is unchanged: ModelPixelScaleTag + ModelTiepointTag are still emitted bit-for-bit identically.
  • Reader was already correct: it prefers ModelTransformationTag when present and falls back to scale + tiepoint otherwise.
  • write_vrt: also fixed dst_y_off / dst_x_off to use the top-left corner of each source's geographic extent rather than origin_y / origin_x directly. 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 survive
  • test_ascending_y_roundtrip -- ascending y coords survive
  • test_descending_x_and_ascending_y_roundtrip -- both axes flipped
  • test_north_up_still_uses_pixel_scale_and_tiepoint -- standard orientation keeps the existing tag layout
  • test_descending_x_uses_transformation_tag -- non-standard orientation emits 34264 and omits 33550 / 33922
  • test_ascending_y_uses_transformation_tag -- same check on the y axis

Full geotiff test suite passes apart from two pre-existing failures (test_window_clamps_to_raster_bounds, test_window_clamps_negative_offsets) that fail on main and are unrelated.

Closes #1716

@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 12, 2026
@brendancol brendancol requested a review from Copilot May 12, 2026 19:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 of ModelPixelScaleTag + ModelTiepointTag when the transform has descending x or ascending y.
  • Teach the writer to serialize ModelTransformationTag (16 doubles) into IFDs for both regular and streaming writes.
  • Fix write_vrt destination offsets to use each source’s true top/left edges (works for ascending-y sources) 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.
@brendancol brendancol merged commit 995b688 into main May 12, 2026
10 of 11 checks passed
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.

geotiff: to_geotiff loses sign of pixel scale for descending x or ascending y

2 participants