Skip to content

Commit 595ece8

Browse files
authored
Fix write_vrt and write_geotiff_gpu signature/docstring drift vs to_geotiff (#1631) (#1633)
* Fix write_vrt and write_geotiff_gpu signature/docstring drift vs to_geotiff (#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. * Update sweep-api-consistency-state for geotiff (#1631) * Address Copilot review on #1633 - 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. * Fix Windows tempfile cleanup PermissionError in test_signature_parity 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.
1 parent 69a75da commit 595ece8

3 files changed

Lines changed: 192 additions & 12 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
module,last_inspected,issue,severity_max,categories_found,notes
2-
geotiff,2026-05-11,1605,HIGH,1;5,"Filed open_geotiff(gpu=True) silently dropping window/band (HIGH, #1605) and fix PR -- adds window+band kwargs to read_geotiff_gpu and forwards them through open_geotiff GPU dispatch. Prior issues 1560/1561/1562 from 2026-05-10 audit all CLOSED. MEDIUM: read_geotiff_dask VRT defensive fallback drops window/band/max_pixels at line 1463 (acceptable since open_geotiff routes earlier and direct callers can switch to read_vrt). write_vrt wrapper uses **kwargs instead of explicit signature -- docs list the args but inspect.signature does not; cosmetic."
2+
geotiff,2026-05-11,1631,MEDIUM,3,"Filed write_vrt and write_geotiff_gpu signature/docstring drift vs to_geotiff (MEDIUM, #1631). Fix in PR (TBD): explicit write_vrt(relative, crs_wkt, nodata) signature (was **kwargs); 'cubic' added to write_geotiff_gpu overview_resampling docstring; write_geotiff_gpu(data) typed xr.DataArray|cupy.ndarray to match to_geotiff. Prior 1605/1606/1611/1612/1613/1615/1623 all CLOSED."
33
reproject,2026-05-10,1570,HIGH,2;5,"Filed cross-module attrs['vertical_crs'] type collision (string vs EPSG int) vs xrspatial.geotiff. Fixed in PR (TBD): reproject now writes EPSG int and preserves friendly token under vertical_datum. MEDIUM kwarg-order drift (transform_precision vs chunk_size) and missing type hints vs geotiff documented but not fixed (cosmetic, kwarg-only)."

xrspatial/geotiff/__init__.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2672,7 +2672,8 @@ def _read_once():
26722672
return result
26732673

26742674

2675-
def write_geotiff_gpu(data, path: str, *,
2675+
def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
2676+
path: str, *,
26762677
crs: int | str | None = None,
26772678
nodata=None,
26782679
compression: str = 'zstd',
@@ -2701,8 +2702,10 @@ def write_geotiff_gpu(data, path: str, *,
27012702
27022703
Parameters
27032704
----------
2704-
data : xr.DataArray (CuPy-backed) or cupy.ndarray
2705-
2D raster on GPU.
2705+
data : xr.DataArray (CuPy- or NumPy-backed), cupy.ndarray, or np.ndarray
2706+
2D or 3D raster. CuPy-backed inputs stay on device; NumPy/Dask
2707+
inputs are uploaded via ``cupy.asarray(np.asarray(data))``
2708+
before compression (matches ``to_geotiff`` parity).
27062709
path : str
27072710
Output file path.
27082711
crs : int, str, or None
@@ -2735,7 +2738,10 @@ def write_geotiff_gpu(data, path: str, *,
27352738
halving until the smallest overview fits in a single tile.
27362739
overview_resampling : str
27372740
Resampling method for overviews: 'mean' (default), 'nearest',
2738-
'min', 'max', 'median', or 'mode'.
2741+
'min', 'max', 'median', 'mode', or 'cubic'. ``mode`` and
2742+
``cubic`` fall back to the CPU implementation in
2743+
``xrspatial.geotiff._writer`` so the GPU writer produces the
2744+
same overview bytes as the CPU writer.
27392745
bigtiff : bool or None
27402746
Force BigTIFF (64-bit offsets). None auto-promotes when the
27412747
estimated file size would exceed the classic-TIFF 4 GB limit.
@@ -3196,7 +3202,10 @@ def _sentinel_for_dtype(nodata_val, dtype):
31963202
return result
31973203

31983204

3199-
def write_vrt(vrt_path: str, source_files: list[str], **kwargs) -> str:
3205+
def write_vrt(vrt_path: str, source_files: list[str], *,
3206+
relative: bool = True,
3207+
crs_wkt: str | None = None,
3208+
nodata: float | None = None) -> str:
32003209
"""Generate a VRT file that mosaics multiple GeoTIFF tiles.
32013210
32023211
Parameters
@@ -3217,14 +3226,18 @@ def write_vrt(vrt_path: str, source_files: list[str], **kwargs) -> str:
32173226
-------
32183227
str
32193228
Path to the written VRT file.
3220-
3221-
Notes
3222-
-----
3223-
Only the keyword arguments listed above are accepted. Passing any
3224-
other keyword raises ``TypeError`` from the underlying writer.
32253229
"""
3230+
# Explicit signature (previously ``**kwargs``) so ``inspect.signature``,
3231+
# IDE autocomplete, and ``mypy --strict`` can see the accepted kwargs
3232+
# without parsing the docstring. Mirrors ``_vrt.write_vrt`` exactly; if
3233+
# that signature changes, this wrapper must be updated in lockstep.
32263234
from ._vrt import write_vrt as _write_vrt_internal
3227-
return _write_vrt_internal(vrt_path, source_files, **kwargs)
3235+
return _write_vrt_internal(
3236+
vrt_path, source_files,
3237+
relative=relative,
3238+
crs_wkt=crs_wkt,
3239+
nodata=nodata,
3240+
)
32283241

32293242

32303243
def plot_geotiff(da: xr.DataArray, **kwargs):
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
"""Regression test for #1631: public write_vrt / write_geotiff_gpu
2+
signature and docstring parity vs to_geotiff.
3+
4+
Three drifts were flagged by the api-consistency sweep on 2026-05-11:
5+
6+
1. ``write_vrt(vrt_path, source_files, **kwargs)`` swallowed every kwarg
7+
into ``**kwargs``. The docstring documented ``relative``, ``crs_wkt``,
8+
``nodata``, but ``inspect.signature`` and IDE autocomplete saw nothing.
9+
2. ``write_geotiff_gpu``'s ``overview_resampling`` docstring omitted
10+
``'cubic'``; ``to_geotiff`` lists it and ``make_overview_gpu`` accepts
11+
it (falling back to CPU).
12+
3. ``write_geotiff_gpu(data, ...)`` lacked the type hint that
13+
``to_geotiff(data, ...)`` has.
14+
15+
This module pins each of those three guarantees against future drift.
16+
"""
17+
from __future__ import annotations
18+
19+
import importlib.util
20+
import inspect
21+
import os
22+
23+
import numpy as np
24+
import pytest
25+
import xarray as xr
26+
27+
from xrspatial.geotiff import (
28+
open_geotiff,
29+
to_geotiff,
30+
write_geotiff_gpu,
31+
write_vrt,
32+
)
33+
34+
35+
def _gpu_available() -> bool:
36+
"""True when cupy imports and CUDA is initialised."""
37+
if importlib.util.find_spec("cupy") is None:
38+
return False
39+
try:
40+
import cupy
41+
42+
return bool(cupy.cuda.is_available())
43+
except Exception:
44+
return False
45+
46+
47+
_HAS_GPU = _gpu_available()
48+
_gpu_only = pytest.mark.skipif(
49+
not _HAS_GPU, reason="cupy + CUDA required",
50+
)
51+
52+
53+
def test_write_vrt_signature_exposes_documented_kwargs():
54+
"""``inspect.signature(write_vrt)`` reports the three accepted kwargs.
55+
56+
Prior to #1631 the public wrapper used ``**kwargs``, so
57+
``inspect.signature`` only saw ``vrt_path`` and ``source_files``.
58+
"""
59+
sig = inspect.signature(write_vrt)
60+
params = sig.parameters
61+
assert 'relative' in params
62+
assert 'crs_wkt' in params
63+
assert 'nodata' in params
64+
# Defaults must match _vrt.write_vrt
65+
assert params['relative'].default is True
66+
assert params['crs_wkt'].default is None
67+
assert params['nodata'].default is None
68+
# No more catch-all VAR_KEYWORD
69+
kinds = {p.kind for p in params.values()}
70+
assert inspect.Parameter.VAR_KEYWORD not in kinds
71+
72+
73+
def test_write_vrt_unknown_kwarg_rejected_at_public_level(tmp_path):
74+
"""A typo'd kwarg now raises ``TypeError`` from the public function
75+
rather than from deep inside ``_vrt.write_vrt``.
76+
"""
77+
arr = np.zeros((8, 8), dtype=np.float32)
78+
da = xr.DataArray(
79+
arr, dims=['y', 'x'],
80+
coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)},
81+
attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)},
82+
)
83+
tif_path = str(tmp_path / 't.tif')
84+
to_geotiff(da, tif_path)
85+
86+
with pytest.raises(TypeError, match='typo_kwarg'):
87+
write_vrt(str(tmp_path / 't.vrt'), [tif_path], typo_kwarg=1)
88+
89+
90+
def test_write_vrt_accepts_documented_kwargs(tmp_path):
91+
"""Each documented kwarg round-trips through the explicit signature."""
92+
arr = np.zeros((8, 8), dtype=np.float32)
93+
da = xr.DataArray(
94+
arr, dims=['y', 'x'],
95+
coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)},
96+
attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)},
97+
)
98+
tif_path = str(tmp_path / 't.tif')
99+
to_geotiff(da, tif_path)
100+
101+
vrt_path = str(tmp_path / 't.vrt')
102+
out = write_vrt(
103+
vrt_path, [tif_path],
104+
relative=False, crs_wkt=None, nodata=-9999.0,
105+
)
106+
assert out == vrt_path
107+
assert os.path.exists(vrt_path)
108+
109+
110+
def test_write_geotiff_gpu_docstring_lists_cubic():
111+
"""``overview_resampling`` docstring includes ``'cubic'`` so it
112+
matches ``to_geotiff`` and the underlying ``make_overview_gpu``.
113+
"""
114+
doc = write_geotiff_gpu.__doc__
115+
assert doc is not None
116+
# Find the overview_resampling block
117+
assert 'overview_resampling' in doc
118+
# The block must mention cubic
119+
block_start = doc.index('overview_resampling')
120+
block_end = doc.index('bigtiff', block_start)
121+
block = doc[block_start:block_end]
122+
assert 'cubic' in block
123+
124+
125+
def test_write_geotiff_gpu_data_has_type_hint():
126+
"""``data`` parameter is annotated, matching ``to_geotiff(data, ...)``.
127+
128+
The annotation also covers ``np.ndarray`` because the implementation
129+
accepts numpy inputs (uploaded via ``cupy.asarray(np.asarray(data))``)
130+
and the test suite exercises that path (e.g.
131+
``test_backend_kwarg_parity_1561.py`` passes a numpy ``dummy``).
132+
"""
133+
sig = inspect.signature(write_geotiff_gpu)
134+
data_param = sig.parameters['data']
135+
assert data_param.annotation is not inspect.Parameter.empty
136+
# The annotation is a forward reference under ``from __future__ import
137+
# annotations``; just confirm it mentions the documented types.
138+
ann_str = str(data_param.annotation)
139+
assert 'DataArray' in ann_str
140+
assert 'cupy' in ann_str
141+
assert 'ndarray' in ann_str # numpy parity vs to_geotiff
142+
143+
144+
@_gpu_only
145+
def test_write_geotiff_gpu_cubic_overview_round_trip(tmp_path):
146+
"""``overview_resampling='cubic'`` works on the GPU writer.
147+
148+
Sanity check that the docstring update is not advertising an
149+
unsupported codec. ``make_overview_gpu`` falls back to the CPU
150+
cubic implementation for parity with the CPU writer.
151+
"""
152+
import cupy
153+
154+
arr_cpu = np.random.RandomState(0).rand(256, 256).astype(np.float32)
155+
arr_gpu = cupy.asarray(arr_cpu)
156+
da_gpu = xr.DataArray(
157+
arr_gpu, dims=['y', 'x'],
158+
coords={'y': np.arange(256.0, 0, -1), 'x': np.arange(256.0)},
159+
)
160+
path = str(tmp_path / 'cog.tif')
161+
write_geotiff_gpu(
162+
da_gpu, path,
163+
cog=True, tile_size=64, overview_resampling='cubic',
164+
)
165+
# Overview level 1 = 1/2 resolution
166+
ov = open_geotiff(path, overview_level=1)
167+
assert ov.shape == (128, 128)

0 commit comments

Comments
 (0)