Skip to content

geotiff: validate coord regularity in _coords_to_transform (#1720)#1726

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

geotiff: validate coord regularity in _coords_to_transform (#1720)#1726
brendancol merged 1 commit into
mainfrom
issue-1720

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

_coords_to_transform used x[1] - x[0] and y[1] - y[0] as the affine pixel sizes without checking that the rest of the spacing matched. GeoTIFF only supports an affine transform, so non-uniform coords can't be expressed faithfully. The writer silently used the first-step spacing and produced a wrong transform.

Repro

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

# x is non-uniform: steps are 1, 1, 1, 2
x = np.array([0.0, 1.0, 2.0, 3.0, 5.0])
y = np.linspace(0.0, 4.0, 5)
da = xr.DataArray(np.zeros((5, 5), np.uint8), dims=['y', 'x'], coords={'y': y, 'x': x})
to_geotiff(da, '/tmp/bad.tif')  # silently writes pixel_width=1.0
rt = open_geotiff('/tmp/bad.tif')
# rt.x.values[4] is 4.0, but the source had 5.0: silent data loss

Fix

Validate np.diff(x) and np.diff(y) against the median step with a 1e-6 relative tolerance in _coords_to_transform. Raise ValueError naming the offending dim and the worst residual. Median (not mean) keeps a single bad sample from shifting the reference step.

Tolerance rationale

1e-6 relative is forgiving for float artifacts in otherwise-uniform coords (e.g. np.linspace round-trip jitter is well below 1e-12) but strict enough to catch real non-uniform data. The tolerance is in the error message so the next maintainer can see why their array failed.

Tests

New xrspatial/geotiff/tests/test_coord_regularity_1720.py:

  • Uniform coords write successfully (no regression).
  • Non-uniform x raises ValueError, message names x.
  • Non-uniform y raises ValueError, message names y.
  • Float jitter at 1e-7 scale within tolerance writes successfully.
  • Jitter at 1e-5 scale above tolerance raises.
  • Two-sample coords pass trivially.
  • Constant coords raise (step == 0 branch).

Also fixed test_pixel_is_point_round_trip, whose hand-typed coords had ~3.6e-3 relative deviation between steps. That is exactly the silent-wrong-transform case the new check catches. Switched to np.linspace.

Closes #1720

@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:07
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

This PR prevents to_geotiff from silently writing an incorrect affine geotransform when x/y coordinates are non-uniformly spaced (GeoTIFF can only represent affine transforms). It adds a regularity check in _coords_to_transform and introduces regression tests covering non-uniform coords and tolerance for small floating-point jitter.

Changes:

  • Add spacing-regularity validation for x/y coords in _coords_to_transform, raising ValueError on non-uniform or constant coords.
  • Add a dedicated regression test module for issue #1720 covering uniform spacing, non-uniform spacing, jitter tolerance, and constant coords.
  • Fix an existing round-trip test to use truly uniform coordinates (switch to np.linspace) to comply with the new validation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
xrspatial/geotiff/__init__.py Adds coord regularity validation to avoid writing silently-wrong affine transforms.
xrspatial/geotiff/tests/test_coord_regularity_1720.py New regression tests for uniform/non-uniform coords, jitter tolerance, and constant coords.
xrspatial/geotiff/tests/test_edge_cases.py Updates PixelIsPoint round-trip test coords to be truly uniform under the new validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +310 to +313
step = float(np.median(diffs))
if step == 0:
raise ValueError(
f"{name} coords are constant; cannot infer pixel size"
Comment on lines +323 to 327
_is_regular(x, "x")
_is_regular(y, "y")

pixel_width = float(x[1] - x[0])
pixel_height = float(y[1] - y[0])
`_coords_to_transform` used `x[1] - x[0]` and `y[1] - y[0]` as the
affine pixel sizes without checking that the rest of the spacing
matched. GeoTIFF only supports an affine transform, so non-uniform
coords cannot be expressed faithfully; the writer silently used the
first-step spacing and produced a wrong transform.

Validate `np.diff` against the median step with a 1e-6 relative
tolerance and raise `ValueError` naming the offending dim and the
worst residual. Median (not mean) keeps a single bad sample from
shifting the reference step.

Also fix `test_pixel_is_point_round_trip`, whose hand-typed coords
had ~3.6e-3 relative deviation and were exactly the silent-wrong-
transform case the new check now catches.

Closes #1720
@brendancol brendancol merged commit 4a48baf into main May 12, 2026
10 of 11 checks passed
@brendancol brendancol deleted the issue-1720 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 trusts coord regularity, silently writes wrong transform for non-uniform coords

2 participants