Annotate window / path / on_gpu_failure on public geotiff API (#1654)#1658
Merged
Conversation
The api-consistency sweep on 2026-05-12 found the same parameter is annotated on some siblings but not on others across the public xrspatial.geotiff surface. Pin each annotation so type-checkers and IDEs validate user code consistently. - window: tuple | None on open_geotiff and read_vrt (read_geotiff_dask and read_geotiff_gpu already had it). - path: str | BinaryIO on to_geotiff and write_geotiff_gpu. write_vrt stays str-only because VRT writes are path-only by design. - on_gpu_failure: str on open_geotiff and read_geotiff_gpu. The deprecated gpu alias on read_geotiff_gpu carries the same str hint. Annotation-only change; no runtime behaviour, defaults, or kwarg renames. BinaryIO is imported under TYPE_CHECKING so the runtime import cost stays at zero with from __future__ import annotations. test_signature_annotations_1654.py pins each annotation to guard against future signature drift. Also updates the api-consistency sweep state CSV.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds missing type annotations on several public xrspatial.geotiff entry points to keep sibling APIs consistent for IDEs/type-checkers, and adds regression tests to prevent future annotation drift.
Changes:
- Annotate
window: tuple | Noneonopen_geotiffandread_vrt. - Annotate writer
path: str | BinaryIOonto_geotiffandwrite_geotiff_gpu. - Annotate GPU failure-policy kwargs
on_gpu_failure: str(and deprecatedgpu: stralias) on GPU-related readers, plus add tests that pin these annotations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xrspatial/geotiff/__init__.py |
Adds the missing public-signature annotations (window, writer path, GPU failure-policy kwargs). |
xrspatial/geotiff/tests/test_signature_annotations_1654.py |
New regression tests that assert the annotations are present and match expected forms. |
.claude/sweep-api-consistency-state.csv |
Updates recorded sweep state to reflect closing #1654. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+131
to
+133
| # window is a 4-tuple; on_gpu_failure must not be passed on | ||
| # gpu=False, so just verify window kwarg roundtrip | ||
| r = open_geotiff(path, window=(0, 0, 4, 4)) |
Comment on lines
+112
to
+114
| def test_open_geotiff_window_and_failure_kwargs_runtime(): | ||
| """The annotated kwargs still accept the documented values at runtime.""" | ||
| import os |
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 #1654.
Summary
The api-consistency sweep on 2026-05-12 found one MEDIUM drift across the public
xrspatial.geotiffsurface: the same parameter is annotated on some siblings but not on others. Adds the missing annotations.window: tuple | Noneonopen_geotiffandread_vrt.read_geotiff_daskandread_geotiff_gpualready had it.path: str | BinaryIOonto_geotiffandwrite_geotiff_gpu.write_vrt.vrt_pathstaysstrbecause VRT writes are path-only by design.on_gpu_failure: stronopen_geotiffandread_geotiff_gpu. The deprecatedgpualias onread_geotiff_gpucarries the samestrhint.BinaryIOis imported underTYPE_CHECKINGso the runtime import cost stays at zero.from __future__ import annotationskeeps all annotations stringified.Not a breaking change
Annotation-only -- no runtime behaviour changes, no defaults changed, no kwarg renames. The sentinel objects (
_ON_GPU_FAILURE_SENTINEL,_GPU_DEPRECATED_SENTINEL) are still used as the runtime default; thestrannotation describes the user-supplied value, not the sentinel.Test plan
pytest xrspatial/geotiff/tests/test_signature_annotations_1654.py-- 11 passing tests pin each new annotation.pytest xrspatial/geotiff/tests/test_signature_parity_1631.py-- 6 prior signature-parity tests still pass.