Cover write_vrt / read_geotiff_gpu(dtype) / write_geotiff_gpu(bigtiff) kwargs#1656
Merged
Merged
Conversation
…) kwargs (test-coverage sweep pass 9) Close three Cat 4 MEDIUM parameter-coverage gaps plus one Cat 4 LOW error path identified by the 2026-05-12 test-coverage sweep on the geotiff module. write_vrt's documented kwargs (relative/crs_wkt/nodata) were pinned by an existing accepted-kwargs smoke test but no test verified the override effect. A regression dropping any override branch and silently using the default-from-first-source would not surface. Verify the XML output for each kwarg directly: relativeToVRT attribute + on-disk source path text for relative=, distinct WKT marker for crs_wkt=, NoDataValue element for nodata=. Parse the VRT back through parse_vrt to assert round-trips. read_geotiff_gpu(dtype=) cast had zero direct tests. The eager CPU path has TestDtypeEager (float64->float32, uint16->int32, uint16->uint8, float-to-int raise, dtype=None preserves native); the dask path has TestDtypeDask. The GPU branch had no equivalent. Mirror the eager matrix on the GPU read path plus the dask+GPU chunks branch and the open_geotiff(gpu=True, dtype=) dispatcher. write_geotiff_gpu(bigtiff=) threads force_bigtiff through to _assemble_tiff but no test asserted the on-disk header switched. The CPU writer had it via test_features::test_force_bigtiff_via_public_api. Add header parse_header().is_bigtiff checks for bigtiff=True/False/None on write_geotiff_gpu plus the to_geotiff(gpu=True) dispatcher. write_vrt(source_files=[]) ValueError error path was uncovered; add it for completeness. 26 tests, all passing on GPU host.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds targeted tests to close several GeoTIFF kwarg coverage gaps (VRT writer kwarg effects, GPU dtype casting, GPU BigTIFF forcing) and records the sweep state update—no production/source code changes.
Changes:
- Add a new test module asserting
write_vrt(relative= / crs_wkt= / nodata=)behavioral effects by inspecting/parsing on-disk VRT XML. - Add GPU-only dtype-cast coverage for
read_geotiff_gpu(dtype=...), including dispatcher paths (open_geotiff(gpu=True, dtype=...)) and the dask+GPUchunks=branch. - Add GPU-only BigTIFF header assertions for
write_geotiff_gpu(bigtiff=...)andto_geotiff(..., gpu=True, bigtiff=True)dispatch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12.py |
New tests covering VRT kwarg effects, GPU dtype casting/dispatch, GPU BigTIFF forcing, and an empty-source_files error path. |
.claude/sweep-test-coverage-state.csv |
Updates sweep tracking log for the 2026-05-12 pass. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+229
to
+245
| """No override means the first source's WKT is used. The source | ||
| was written with crs=4326, so the WKT must mention EPSG:4326's | ||
| marker text (any non-empty WKT is the contract; we check it is | ||
| not the override value to defend against silent default | ||
| substitution by the writer).""" | ||
| vrt_path = str(tmp_path / 'crs_wkt_default.vrt') | ||
| write_vrt(vrt_path, [source_tif]) | ||
| parsed = self._read_parsed(vrt_path, tmp_path) | ||
| # The first source has crs=4326. The WKT will be something | ||
| # non-empty mentioning EPSG:4326 (precise WKT text depends on | ||
| # the EPSG database wired into the writer). | ||
| if parsed.crs_wkt is not None: | ||
| # When the WKT is present it must come from the source | ||
| # (mentions a geographic marker) rather than the override. | ||
| text = parsed.crs_wkt.lower() | ||
| assert 'unittest_override_sweep_2026_05_12' not in text | ||
|
|
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
Test-coverage gap sweep on the geotiff module surfaced three MEDIUM parameter-coverage gaps and one LOW error-path gap. Tests only; no source changes.
write_vrt(relative=/crs_wkt=/nodata=): existing smoke test pins that the kwargs are accepted; nothing checked the override actually took effect. Inspect the on-disk XML directly for each kwarg.read_geotiff_gpu(dtype=): zero direct tests. Eager CPU and dask each have a full TestDtype matrix; GPU had nothing. Mirror that matrix on the GPU read path, the dask+GPU chunks branch, and theopen_geotiff(gpu=True, dtype=)dispatcher.write_geotiff_gpu(bigtiff=):force_bigtiff=is threaded to_assemble_tiffbut no test asserted the on-disk header byte switched. Pinparse_header().is_bigtifffor the GPU writer and theto_geotiff(gpu=True)dispatcher.write_vrt(source_files=[])ValueError was uncovered.26 tests, all passing on a GPU host.
Test plan