geotiff tests: consolidate georef and rotation cluster (#2435)#2463
Merged
Conversation
…b#2435) Fold twelve top-level georef / no-georef / rotation test files into xrspatial/geotiff/tests/read/test_georef.py. Read-side and write-side cases share the rotated-affine and no-georef-marker fixtures, so the write-side tests stay in the same module under a write-side heading rather than splitting into write/test_georef.py. GPU gating moves to the shared requires_gpu marker and gpu_available probe from _helpers/markers.py. Per-issue rotated-TIFF and VRT writers keep their distinct byte layouts and are suffixed by issue number to avoid collision. The status suite imports _write_rotated_tiff from read/test_crs.py, which is left untouched. CLUSTER_AUDIT_GEOREF.md maps every old file to its new section; it is deleted in a pre-merge commit per epic xarray-contrib#2424.
brendancol
commented
May 26, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: geotiff tests: consolidate georef and rotation cluster (#2435)
Tests-only consolidation. Twelve top-level files fold into read/test_georef.py. No source under xrspatial/geotiff/ changed, so the algorithm / backend-dispatch / performance sections of the checklist do not apply. I verified the move preserved every test rather than re-deriving correctness.
Blockers (must fix before merge)
None.
Suggestions (should fix, not blocking)
None.
Nits (optional improvements)
read/test_georef.py:81imports_write_rotated_tiff as _write_rotated_tiff_crswith a# noqa: E402, but the import sits in the normal top-of-file block, not after code. flake8 with--select=E402is clean without it, so the marker can be dropped.
What looks good
- Test count is identical: 144 unique test functions in both the old twelve files and the new module, 171 collected items each.
commshows zero names dropped. - Every assertion is carried over verbatim; the only changes are mechanical (helper renames, shared marker).
- Helper-name collisions across the source files (
_write_rotated_tiff,_write_rotated_vrt,_make_da,_rotated_dataarray,_ROTATED_TUPLE) are resolved by suffixing each with its issue number, so the distinct byte layouts stay distinct. - The status suite keeps importing
_write_rotated_tifffromread/test_crs.pywith the same(path, arr)call shape; the imported helper's signature is unchanged, so the dependency holds andread/test_crs.pyis untouched. _NO_GEOREF_KEYresolves to the same string from_coordsand_geotags, so consolidating the import to_coordsdoes not change any marker comparison.- GPU gating moved to the shared
requires_gpumarker; the one spot that read the old_HAS_GPUglobal now callsgpu_available(), which returns the same bool. - flake8 is clean; full suite collects 5890 with no errors.
Checklist
- [n/a] Algorithm matches reference/paper -- no algorithm changed
- All implemented backends produce consistent results -- GPU + dask + numpy paths preserved, all pass on this checkout
- NaN handling is correct -- unchanged from source files
- Edge cases are covered by tests -- 1x1 / Nx1 / 1xN / no-georef / rotated all carried over
- Dask chunk boundaries handled correctly -- unchanged
- No premature materialization or unnecessary copies -- tests-only
- [n/a] Benchmark exists or is not needed -- tests-only
- [n/a] README feature matrix updated -- no new function
- Docstrings present and accurate -- module + per-section docstrings explain provenance
brendancol
commented
May 26, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review (round 2)
The one nit from the first pass is fixed in 58bd26c: the unnecessary # noqa: E402 on the read/test_crs import is gone, and flake8 stays clean.
The audit file CLUSTER_AUDIT_GEOREF.md is deleted in 1e5c023 per epic #2424. Nothing else outstanding; the diff is now tests-only with no leftover scaffolding. 171 tests still pass on this checkout.
brendancol
added a commit
that referenced
this pull request
May 26, 2026
The georef/rotation cluster (#2463) moved test_no_georef_windowed_coords_1710.py into read/test_georef.py but left the reader.windowed row in release_gate_geotiff.rst citing the deleted path. Repoint it so the checklist-parity gate stays green on the merged branch.
brendancol
added a commit
that referenced
this pull request
May 26, 2026
* geotiff tests: consolidate VRT tail cluster (#2437) Fold the 12 VRT-residue files into existing vrt/ modules plus a new vrt/test_parity.py: - missing-sources trio (1843 internal entry point + STRICT env, 1860 public default, 1799/2265 chunked policy) -> vrt/test_missing_sources.py - srcrect-validation (1784), kwarg-drop (1685), tiled-validation (1862) -> vrt/test_validation.py - lazy-chunks (1798), dask-vrt-kwargs (1795) -> vrt/test_window.py - backend-parity (2321), finalization-parity (2162), backend-coverage (2026_05_11) -> new vrt/test_parity.py Update release_gate_geotiff.rst rows to cite the consolidated homes so the checklist-parity gate stays green. CLUSTER_AUDIT_VRT_TAIL.md maps every old file::test to its new home and is deleted before merge. * Repoint stale release-gate citation after merge with #2463 (#2437) The georef/rotation cluster (#2463) moved test_no_georef_windowed_coords_1710.py into read/test_georef.py but left the reader.windowed row in release_gate_geotiff.rst citing the deleted path. Repoint it so the checklist-parity gate stays green on the merged branch. * Delete cluster audit doc before merge (#2437)
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 #2435 (cluster 11 of the long-tail GeoTIFF test consolidation epic #2424). Tests-only.
Summary
Fold twelve top-level georef / no-georef / rotation test files into a single
xrspatial/geotiff/tests/read/test_georef.py, grouped by concern:Read side:
test_georef_edges.py(Add geotiff edge-case tests: IFD chain loops, header parsing, descending-y, non-georeferenced #1482) -- non-georef reads, south-up writestest_georef_resolver_parity_2211.py(Refactor GeoTIFF Phase 2: centralize transform/georef contract (PR-B of #2211) #2225) --resolve_georefparity across reader / writer call sitestest_georef_status_2136.py(geotiff: canonical georef_status attribute to disambiguate CRS vs transform presence #2136) -- canonicalgeoref_statusattr for the five reader statestest_no_georef_attr_migration_2133.py(geotiff: finish migration from integer coord sentinel to explicit no-georef attr #2133) -- no-georef marker migration from coord dtype to attrstest_no_georef_marker_2120.py(geotiff: to_geotiff silently strips georef on int64 step-1 user coords #2120) -- int64 step-1 user grids keep their georeftest_no_georef_windowed_coords_1710.py(open_geotiff windowed reads of non-georef TIFFs produce float64 half-pixel-shifted coords #1710) -- windowed reads emit the same integer pixel coords as full readstest_rotated_transform_attr_1764.py(to_geotiff silently drops rotation/shear from attrs['transform'] #1764) --_transform_from_attrrejects rotated / sheared 6-tuplestest_rotated_affine_attr_2129.py(geotiff: surface rotated_affine 6-tuple on the DataArray attrs #2129) --allow_rotated=Truesurfacesattrs['rotated_affine']test_rotated_typed_error_2267.py(Rotated GeoTIFF errors bypass the typed RotatedTransformError exception #2267) -- GeoTIFF and VRT rotated reads raise the same typed errorWrite side (kept in the same file under a write-side heading because they share the rotated-affine and no-georef-marker fixtures; splitting into write/test_georef.py would scatter one concern across two modules):
test_degenerate_georef_1945.py(geotiff: 1xN / Nx1 georeferenced writes silently strip transform #1945) -- 1xN / Nx1 / 1x1 georeferenced writes preserve their transformtest_no_georef_writer_round_trip_1949.py(geotiff: to_geotiff turns no-georef int64 pixel coords into a fake transform on round-trip #1949) -- no-georef files keep integer coords across read/write/readtest_to_geotiff_drop_rotation_2216.py(to_geotiff silently drops rotated_affine instead of failing closed #2216) -- to_geotiff refuses to silently drop a rotated affineGPU gating moves to the shared
requires_gpumarker andgpu_availableprobe from_helpers/markers.py. The per-issue rotated-TIFF and VRT writers keep their distinct byte layouts and are suffixed by issue number to avoid collision. The status suite imports_write_rotated_tifffromread/test_crs.py, which owns the rotated-CRS read surface and is left untouched. No assertion changed.Audit
CLUSTER_AUDIT_GEOREF.mdmaps every old file to its new section. The audit file is deleted in a final pre-merge commit.Verification
pytest xrspatial/geotiff/tests/read/test_georef.py -q: 171 passed (same total as the twelve source files).pytest xrspatial/geotiff/tests/read/ -q: 711 passed, 4 skipped.pytest xrspatial/geotiff/tests/ --co -q: 5890 collected, no errors.Test plan