Update y/x coords for mirror-flip TIFF orientations (#1537)#1539
Merged
brendancol merged 3 commits intomainfrom May 11, 2026
Merged
Update y/x coords for mirror-flip TIFF orientations (#1537)#1539brendancol merged 3 commits intomainfrom
brendancol merged 3 commits intomainfrom
Conversation
Orientations 2/3/4 in the TIFF Orientation tag (274) flip the array horizontally, both axes, or vertically. PR #1521 applied the buffer remap but left the y/x coord arrays computed from the un-flipped transform, so xarray label lookups against a georeferenced file returned the wrong pixel under any of those orientations. The fix updates the geo_info transform after the array remap so a displayed pixel and its geographic label point to the same location on the ground. PixelIsArea and PixelIsPoint both work; the unrotated transform is still preserved for ungeoreferenced files (coords stay on integer pixel indices).
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes GeoTIFF Orientation tag handling for mirror-flip cases (2/3/4) by ensuring georeferenced x/y coordinates stay aligned with the post-orientation pixel buffer so xarray label-based selection (.sel(x=..., y=...)) returns the correct pixel.
Changes:
- Update
read_to_arrayto adjustGeoTransformorigin and pixel scale signs for orientations 2/3/4 (including correct PixelIsArea vs PixelIsPoint shifts). - Add targeted tests validating coord/pixel fidelity for orientations 2/3/4 under both PixelIsArea and PixelIsPoint, plus a no-georef passthrough check.
- Update the internal sweep tracking CSV entry ordering/content.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
xrspatial/geotiff/_reader.py |
Adjusts geotransform metadata to match mirror-flipped pixel buffers for orientations 2/3/4. |
xrspatial/geotiff/tests/test_orientation.py |
Adds regression tests ensuring .sel() correctness and expected coordinate arrays for orientations 2/3/4. |
.claude/sweep-accuracy-state.csv |
Updates internal audit/state tracking notes for the geotiff sweep. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The previous guard checked `transform is not None`, but `GeoInfo.transform` is a dataclass with a default GeoTransform factory so the check was always true. Plain TIFFs with an Orientation tag and no GeoTIFF tags ended up exposing a fabricated transform via attrs, even though downstream consumers correctly fall back to integer pixel coords for `has_georef=False`. Now `has_georef` gates both the 2/3/4 origin/sign update and the 5-8 axis swap, so the transform attr stays at the dataclass default for non-georef files. Adds `test_orient_2_3_4_no_geo_does_not_modify_transform` to pin the attrs-stay-default behaviour.
brendancol
added a commit
that referenced
this pull request
May 11, 2026
- GPU code now owns orientation handling end-to-end. Discard the geo_info returned by `read_to_array` in all four CPU-fallback paths (stripped, planar=2 cpu_fallback_needed, sparse tile, GPU exception) and re-derive the post-orientation transform from the pre-extracted geo_info via _apply_orientation_geo_info. Works regardless of whether the read_to_array 2/3/4 fix in #1539 has merged yet, and avoids the double-swap that would otherwise happen for orientations 5-8. - Test extratags promoted to 5-tuples (33550/33922/34735) for consistency with test_orientation.py and tifffile's preferred signature.
brendancol
added a commit
that referenced
this pull request
May 11, 2026
* Apply TIFF Orientation tag in read_geotiff_gpu (#1540) PR #1521 wired up the Orientation tag (274) on the CPU read path but left the GPU path on the raw stored buffer, so any TIFF written with orientation 2-8 decoded to different pixel values on CPU vs GPU. This adds two helpers in xrspatial/geotiff/__init__.py: - _apply_orientation_gpu mirrors the CPU _apply_orientation slicing logic against a cupy ndarray (eight orientations, 2-D and 3-D variants). - _apply_orientation_geo_info centralises the post-flip transform update so both read_to_array (CPU) and read_geotiff_gpu update GeoTransform consistently. read_geotiff_gpu now applies orientation post-decode for every pure-GPU code path, and reuses the geo_info returned by read_to_array when the CPU fallback path runs (since that path already applies the remap). A flag (arr_was_cpu_decoded) keeps us from double-applying. Test coverage: test_orientation_gpu.py exercises every orientation on single-band tiled, single-band stripped, and 3-band planar=2 tiled files, plus georef sel-fidelity for the mirror-flip cases. * Address PR #1546 review - GPU code now owns orientation handling end-to-end. Discard the geo_info returned by `read_to_array` in all four CPU-fallback paths (stripped, planar=2 cpu_fallback_needed, sparse tile, GPU exception) and re-derive the post-orientation transform from the pre-extracted geo_info via _apply_orientation_geo_info. Works regardless of whether the read_to_array 2/3/4 fix in #1539 has merged yet, and avoids the double-swap that would otherwise happen for orientations 5-8. - Test extratags promoted to 5-tuples (33550/33922/34735) for consistency with test_orientation.py and tifffile's preferred signature.
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
read_to_arrayso xarray label lookups land on the right pixel for georeferenced files.N * step) and PixelIsPoint (shifts by(N-1) * step).Context
PR #1521 added the buffer remap for the Orientation tag (274). The 5-8 axis-swap branch already updates the transform (and warns). The 2/3/4 mirror branch was never wired up, so
_reader.pyflipped the array but kept the un-flipped origin and pixel-scale signs.da.sel(x=..., y=...)against the result returned whatever pixel originally lived at those coords -- not the pixel actually displayed there. See #1537 for a full reproducer.Test plan
pytest xrspatial/geotiff/tests/test_orientation.py -q(was 36, now 46 -- new tests cover sel-fidelity for orient 2/3/4 under PixelIsArea and PixelIsPoint, the coord-array first-element values, and the no-georef passthrough).pytest xrspatial/geotiff/tests/ -q --no-header --deselect xrspatial/geotiff/tests/test_features.py::TestPalette(942 passed; the deselected test failures are unrelated -- a Python 3.14 / matplotlib deepcopy regression).pytest xrspatial/geotiff/tests/test_planar_multiband.py xrspatial/geotiff/tests/test_lerc_valid_mask_gpu.py xrspatial/geotiff/tests/test_predictor2_big_endian_gpu_1517.py xrspatial/geotiff/tests/test_gpu_byteswap_1508.py xrspatial/geotiff/tests/test_gpu_strict_fallback_1516.py xrspatial/geotiff/tests/test_gpu_stripped_multiband.py -q(91 passed; CPU-only change, GPU path unaffected).Closes #1537.