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
@brendancol brendancol deleted the issue-1716 branch May 15, 2026 04:41
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