Fix write_vrt and write_geotiff_gpu signature/docstring drift vs to_geotiff (#1631)#1633
Merged
brendancol merged 4 commits intoMay 11, 2026
Conversation
…eotiff (xarray-contrib#1631) Three API-consistency drifts surfaced by the sweep on 2026-05-11: 1. write_vrt used **kwargs even though the docstring listed three accepted kwargs (relative, crs_wkt, nodata). inspect.signature, IDE autocomplete, and mypy --strict could not see them. Replaced with an explicit signature that mirrors _vrt.write_vrt and forwards each kwarg by name. A typo'd kwarg now raises TypeError from the public wrapper rather than from deep inside the internal helper. 2. write_geotiff_gpu's overview_resampling docstring omitted 'cubic'. to_geotiff lists it; the underlying make_overview_gpu accepts it (falls back to CPU for parity with the CPU writer). Updated the docstring to match. 3. write_geotiff_gpu(data) was untyped while to_geotiff(data) was annotated xr.DataArray | np.ndarray. Added xr.DataArray | cupy.ndarray for parity. Annotation is a forward reference under the module's `from __future__ import annotations`, so cupy stays lazily imported at function-body level. Regression test: xrspatial/geotiff/tests/test_signature_parity_1631.py pins each guarantee. inspect.signature parity, unknown-kwarg rejection at the public wrapper, cubic-resampling round-trip on the GPU writer.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses API-consistency drift in xrspatial.geotiff by making the public writer entry points’ signatures/docstrings align with their sibling/internal implementations, improving inspect.signature visibility and developer tooling behavior.
Changes:
- Replaced
write_vrt(..., **kwargs)with an explicit keyword-only signature that mirrors_vrt.write_vrt. - Updated
write_geotiff_gpudocstring to include'cubic'foroverview_resamplingand added adatatype annotation. - Added regression tests to pin signature/docstring/type-hint parity guarantees (and updated the sweep state CSV).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xrspatial/geotiff/__init__.py |
Public API signature/docstring adjustments for write_vrt and write_geotiff_gpu. |
xrspatial/geotiff/tests/test_signature_parity_1631.py |
New regression tests covering signature exposure, kwarg rejection, docstring content, type hints, and a GPU cubic overview smoke test. |
.claude/sweep-api-consistency-state.csv |
Updates the API-consistency sweep tracking entry for this issue/PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- test_signature_parity_1631.py: replace dead `skipif(... if False else False)` decorator with the suite's standard cupy+CUDA skip pattern (`_gpu_available()` helper + `_gpu_only` marker with reason `"cupy + CUDA required"`), matching `test_dask_cupy_combined.py`, `test_gpu_window_band_1605.py`, etc. - __init__.py: widen write_geotiff_gpu `data` annotation to include np.ndarray (matches `cupy.asarray(np.asarray(data))` runtime behavior and `to_geotiff(data: xr.DataArray | np.ndarray, ...)` parity); sync the `data:` docstring line and note the upload step. - Strengthen the type-hint test to also assert `ndarray` is present in the annotation so the numpy parity widen is pinned against future drift.
Replace tempfile.TemporaryDirectory with pytest's tmp_path fixture. write_vrt holds an mmap handle on its source GeoTIFF, which Windows refuses to delete during TemporaryDirectory.__exit__. pytest's tmp_path cleanup uses ignore_errors=True, so the same handle quirk no longer breaks the test. Removes unused tempfile import.
This was referenced May 12, 2026
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
Closes #1631. The api-consistency sweep against
xrspatial.geotiffon 2026-05-11 flagged three signature / docstring drifts between sibling writer entry points:write_vrtused**kwargsdespite documenting three concrete kwargs.inspect.signature(write_vrt)returned(vrt_path, source_files, **kwargs)so IDEs, autocomplete, and type checkers could not see them. Replaced with an explicit signature mirroring_vrt.write_vrt. A typo'd kwarg now raisesTypeErrorfrom the public wrapper instead of from deep inside the internal helper.write_geotiff_gpu'soverview_resamplingdocstring omitted'cubic'.to_geotifflists it;make_overview_gpuaccepts it (falls back to CPU for parity with the CPU writer). Synced the docstring.write_geotiff_gpu(data)had no type hint whileto_geotiff(data)was annotatedxr.DataArray | np.ndarray. Addedxr.DataArray | cupy.ndarray. The module already hasfrom __future__ import annotations, so this is a forward reference and cupy stays lazily imported.None of these are correctness changes; they make the public API readable to
inspect.signature, IDE tooling, and type checkers.Regression test
xrspatial/geotiff/tests/test_signature_parity_1631.pypins each guarantee:inspect.signature(write_vrt)exposesrelative,crs_wkt,nodatawith the right defaults.TypeErrorat the public wrapper level.write_geotiff_gpu.__doc__lists'cubic'inside theoverview_resamplingblock.write_geotiff_gpu(data)has an annotation that mentionsDataArrayorcupy.write_geotiff_gpu(..., overview_resampling='cubic', cog=True)round-trips on GPU.Deprecation impact
None. The three changes only:
**kwargs,'cubic'to a docstring (no signature change),from __future__ import annotations.Existing callers using positional or keyword forms continue to work unchanged.
Test plan
pytest xrspatial/geotiff/tests/test_signature_parity_1631.py(6 new tests, all pass)pytest xrspatial/geotiff/tests/(1301 passed, 7 skipped, 3 pre-existing-deselected, no regressions)open_geotiff,read_geotiff_gpu,read_geotiff_dask,read_vrt,to_geotiff,write_geotiff_gpu,write_vrton numpy / dask / cupy backends.