resample: refresh transform to match actual coord orientation (#2571)#2587
Merged
Conversation
The transform refresh block at the end of resample() was hard-coding a north-up rasterio tuple (positive x scale, negative y scale, origin at the top-left edge). For arrays with ascending y or descending x coords that produced an attrs['transform'] that did not match the coords on the same DataArray, so downstream code that georeferenced via attrs['transform'] would get a flipped raster. Use the signed px / py returned by _new_coords (positive when the coord ascends along the axis, negative when it descends) and anchor the origin at *_edge_start, which is already the leading edge of the first row / column on the side of vals[0]. That matches rasterio's `(col=0, row=0) -> first array pixel` convention for every coord orientation. Tests cover all four orientation combinations (x asc/desc x y asc/desc) and assert the refreshed transform round-trips against the output coords.
brendancol
commented
May 28, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: resample transform refresh (#2571)
Blockers
None.
Suggestions
None.
Nits
- xrspatial/tests/test_resample.py:163 -- the
_raster_with_orientation
docstring says "upper-left edge of pixel [0, 0]", but for ascending
y the first pixel sits at the geographic lower-left. The
transform is still right; the wording just borrows rasterio's
"upper-left" convention where "leading-edge corner of pixel [0, 0]"
would be more honest. - xrspatial/tests/test_resample.py:191 --
assert b == 0.0 and d == 0.0
packs two facts into one assertion. Splitting them tells you which
off-diagonal drifted when it fails.
What looks good
- The math holds up.
px/pyout of_new_coordsare already
signed, and*_edge_startis the leading edge on thevals[0]
side, sotransform * (col, row)lands where the output coords
say it should regardless of orientation. - The pre-existing
test_transform_refreshed_on_downsampleis
untouched and still passes, which is a useful regression anchor. - Metadata-only change, so the fix covers numpy, cupy, dask+numpy,
and dask+cupy without any per-backend code. - The parametrized test sweeps all four x asc/desc x y asc/desc
combos and round-tripstransform * (col+0.5, row+0.5)against
the output pixel centers.
Checklist
- Algorithm matches the rasterio Affine convention
- All backends consistent (metadata-only)
- NaN handling unchanged
- Orientation edge cases covered
- Dask chunks N/A
- No premature materialization
- Benchmark not needed
- README feature matrix not applicable
- Docstrings unchanged
- `_raster_with_orientation` docstring now says "leading-edge corner" rather than "upper-left edge", which only matches when both coords are north-up. - Split `assert b == 0.0 and d == 0.0` into two asserts so a failure identifies which off-diagonal drifted.
brendancol
commented
May 28, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review (#2571)
Both nits from the previous review are fixed in 34dc110:
_raster_with_orientationdocstring now says "leading-edge corner"
instead of "upper-left edge".- The combined off-diagonal assert is split into two.
No new findings on the follow-up pass. The 19
TestMetadataPropagation tests still pass locally.
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.
Closes #2571.
Summary
resample()was always writing a north-up rasterio transform(
+x,-y, top-left origin), even when the input had ascending yor descending x coords. The refreshed
attrs['transform']then nolonger matched the coords on the same DataArray, so downstream
consumers that georeference via
transformwould see a flippedraster.
px/pyfrom_new_coordsand anchor theorigin at
*_edge_start(the leading edge on the side ofvals[0]). This matches rasterio's(col=0, row=0) -> first array pixelconvention for every coord orientation.Backend coverage
The transform refresh runs in pure Python on the metadata after the
backend dispatch, so the fix applies to numpy, cupy, dask+numpy, and
dask+cupy identically. No backend-specific code changed.
Test plan
pytest xrspatial/tests/test_resample.py::TestMetadataPropagation-- 19 passed, includes the 4 new orientation parametrizations.
pytest xrspatial/tests/test_resample.py-- 182 passed.