Pin nodata + streaming_buffer_bytes annotations on writer trio (#1705)#1707
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns public GeoTIFF writer trio type annotations in xrspatial.geotiff by pinning nodata and streaming_buffer_bytes annotations (and, for write_geotiff_gpu, the streaming_buffer_bytes signature default) to remove type-hint drift between sibling APIs.
Changes:
- Annotate
nodataonto_geotiffandwrite_geotiff_gpuasfloat | int | None. - Change
write_geotiff_gpu.streaming_buffer_bytesannotation tointand default to256 * 1024 * 1024to matchto_geotiff. - Add a new regression test module asserting annotation/default parity plus light runtime smoke tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
xrspatial/geotiff/__init__.py |
Pins nodata annotations and aligns write_geotiff_gpu.streaming_buffer_bytes signature with to_geotiff. |
xrspatial/geotiff/tests/test_signature_annotations_1705.py |
Adds regression tests to prevent future annotation/default drift across the writer trio. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path = str(tmp_path / 'nodata_int.tif') | ||
| to_geotiff(da, path, nodata=-9999) | ||
| r = open_geotiff(path) | ||
| assert r.attrs.get('nodata') == -9999 |
| p1 = str(tmp_path / 'default.tif') | ||
| p2 = str(tmp_path / 'override.tif') | ||
| write_geotiff_gpu(da_gpu, p1) | ||
| write_geotiff_gpu(da_gpu, p2, streaming_buffer_bytes=8 * 1024 * 1024) | ||
| # Both files have identical sizes -- the buffer kwarg is a no-op. | ||
| import os | ||
|
|
||
| assert os.path.getsize(p1) == os.path.getsize(p2) |
| bigtiff: bool | None = None, | ||
| max_z_error: float = 0.0, | ||
| streaming_buffer_bytes: int | None = None) -> None: | ||
| streaming_buffer_bytes: int = 256 * 1024 * 1024) -> None: |
| import numpy as np | ||
| import xarray as xr | ||
|
|
Follow-up to #1654. The api-consistency sweep on 2026-05-12 found two remaining annotation gaps across xrspatial.geotiff's writer trio. * nodata: write_vrt got `float | int | None` in #1684, but to_geotiff and write_geotiff_gpu still exposed bare `nodata=None`. Type-checkers inferred Any on the eager writers while inferring float|int|None on the VRT writer, even though all three docstrings describe the same accepted-type set. * streaming_buffer_bytes: `int = 256*1024*1024` on to_geotiff vs `int | None = None` on write_geotiff_gpu. The GPU writer no-ops the kwarg (`del streaming_buffer_bytes`) so the type signature was the only consistency dimension. Pin both to `int = 256*1024*1024` so a caller forwarding the same kwargs to either entry point sees the same hint and default. Annotation-only change; no runtime behaviour, defaults (effective for to_geotiff and the GPU no-op), or kwarg renames. The existing write_geotiff_gpu del-on-entry remains, so the value continues to be ignored on the GPU path. test_signature_annotations_1705.py pins each annotation plus a runtime smoke test for to_geotiff(nodata=int) and a no-op assertion for write_geotiff_gpu(streaming_buffer_bytes=...) so the GPU writer's output stays byte-identical regardless of the kwarg value. Closes #1705.
brendancol
force-pushed
the
deep-sweep-api-consistency-geotiff-2026-05-12-1778611459-2954-01
branch
from
d324ccf to
1554d00
Compare
- Relax nodata read-back assertion to numeric comparison: the reader
parses GDAL_NODATA as a float (`_geotags.extract_geo_info` does
`nodata = float(nodata_str)`), so compare `float(...) == -9999.0`
rather than the raw `== -9999`.
- Pin GPU writer no-op by full file-bytes equality instead of just
file sizes -- the writer is deterministic for a fixed input, so a
regression that produces different bytes at the same size still
trips the assertion.
- Tighten GPU skip to the geotiff test-suite convention:
`pytest.importorskip('cupy')` plus `cupy.cuda.is_available()` check
with the canonical `cupy + CUDA required` reason string.
The streaming_buffer_bytes default change on write_geotiff_gpu
(`int | None = None` -> `int = 256 * 1024 * 1024`) is intentional --
the PR's goal is annotation + default parity with `to_geotiff` so a
caller forwarding the same kwargs to either entry point sees the same
signature. The kwarg is a runtime no-op (deleted on entry) so the
surface default change has no behavioural effect.
|
Addressed the four Copilot review comments in f06720b:
All 7 tests in |
Summary
Follow-up to #1654. The api-consistency sweep on 2026-05-12 found two more annotation gaps across
xrspatial.geotiff's writer trio.nodata:write_vrtgotfloat | int | Nonein geotiff: write_vrt nodata type hint is float-only but to_geotiff/write_geotiff_gpu accept int #1684, butto_geotiffandwrite_geotiff_gpustill exposed barenodata=None. Type-checkers inferredAnyon the eager writers while inferringfloat | int | Noneon the VRT writer, even though all three docstrings describe the same accepted-type set.streaming_buffer_bytes:int = 256*1024*1024onto_geotiffvsint | None = Noneonwrite_geotiff_gpu. The GPU writer no-ops the kwarg (del streaming_buffer_byteson entry) so the type signature was the only consistency dimension. Pin both toint = 256*1024*1024so a caller forwarding the same kwargs to either entry point sees the same hint and default.Annotation-only change; no runtime behaviour, defaults (effective for
to_geotiffand the GPU no-op), or kwarg renames. The GPU writer'sdel-on-entry remains, so the value continues to be ignored on the GPU path.Closes #1705.
Test plan
pytest xrspatial/geotiff/tests/test_signature_annotations_1705.pycovers the new pinned-annotation assertions plus ato_geotiff(nodata=<int>)round-trip smoke test and a no-op assertion on the GPU writer'sstreaming_buffer_byteskwarg.pytest xrspatial/geotiff/tests/test_signature_annotations_1654.pystill passes (the Public geotiff API: type annotations missing on window / path / on_gpu_failure #1654 annotations are unchanged).pytest xrspatial/geotiff/tests/test_signature_parity_1631.pystill passes (__all__andinspect.signaturecontracts unaffected).pytest xrspatial/geotiff/tests/test_features.py -k allstill passes.