geotiff: fix CI regression where int-coord no-georef writes hit fail-closed guard#1968
Closed
brendancol wants to merge 1 commit into
Closed
geotiff: fix CI regression where int-coord no-georef writes hit fail-closed guard#1968brendancol wants to merge 1 commit into
brendancol wants to merge 1 commit into
Conversation
…oref sentinel PR xarray-contrib#1945 added require_transform_for_georeferenced to fail closed when a DataArray carries spatial coords but no transform could be derived. PR xarray-contrib#1949 (merged the same day) made coords_to_transform return None for integer-dtype coords because those are the read-side no-georef sentinel that open_geotiff emits for files without GeoTIFF tags. The combination breaks any round-trip that reads a non-georef TIFF and writes it back: coords_to_transform returns None on the int coords, then require_transform_for_georeferenced raises ValueError. Mirror the int-dtype short-circuit in the fail-closed guard so the sentinel path stays silent and only float-coord 1x1 inputs trip the error.
|
Hi @brendancol, thanks for the PR! Would you mind filing a quick New contributor introduction issue when you get a chance? It helps us point you at issues that fit what you'd like to work on. Most fields are optional. Reviewing your PR doesn't depend on it, just a friendly nudge. |
3 tasks
Contributor
Author
|
Closed — superseded by #1969, which was merged with the same int-coord short-circuit plus a clearer error message and a regression test. Rebasing this branch onto current |
brendancol
added a commit
that referenced
this pull request
May 15, 2026
…1970) The previous gate skipped maintainers via github.event.pull_request.author_association. On PR #1968 that field came through as something other than MEMBER for a maintainer's cross-fork PR (brendancol, admin permission), so the welcome workflow greeted them as a first-time contributor. The same field has misreported same-repo maintainer PRs in the past; copilot-review.yml already gates around it via head.repo == base.repo. Apply the equivalent here and add a runtime collaborator-permission lookup so cross-fork PRs from maintainers' personal forks are also skipped. The default GITHUB_TOKEN has read access to the collaborator-permission endpoint on the repo where the workflow runs, so the call works without additional secrets.
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
1 participant
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
CI on main has been red since the back-to-back merges of #1953 (preserve transform on 1xN / Nx1 georeferenced writes) and #1954 (keep no-georef int coords through to_geotiff round-trip).
require_transform_for_georeferenced, which raises when a DataArray carries spatial coords but no transform was derived.coords_to_transformreturnNonefor integer-dtype coords, because those are the read-side no-georef sentinel the eager reader emits for files without GeoTIFF tags.The interaction breaks any read -> write round-trip of a non-georef TIFF:
coords_to_transformcorrectly returnsNoneon the int coords, then the fail-closed guard sees coords present + no transform and raisesValueError("Cannot infer GeoTIFF transform...").Eleven previously-passing tests started failing on the merge of #1959:
test_features.py::TestExtraTags::test_extra_tags_round_triptest_georef_edges.py::TestNonGeoreferencedRead::test_round_trip_preserves_pixelstest_metadata_round_trip_1484.py(3 tests)test_vrt_int_nodata_1564.py(6 tests)The fix mirrors the int-dtype short-circuit from
coords_to_transforminsiderequire_transform_for_georeferenced: when both spatial coord arrays are integer-dtype, treat that as the no-georef sentinel and return without raising. Float-coord 1x1 inputs still trip the guard, which is what #1945 was actually targeting.Test plan
test_degenerate_georef_1945.py)test_no_georef_writer_round_trip_1949.py)xrspatial/geotiff/suite: 3177 passed (8 unrelated pre-existing GPU failures local to my environment, not in CI)