From 29c215be81e1b6ee5971f59f243ff70391ed5b90 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 11 May 2026 14:17:30 -0700 Subject: [PATCH 1/4] 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. --- xrspatial/geotiff/__init__.py | 28 ++-- .../tests/test_signature_parity_1631.py | 151 ++++++++++++++++++ 2 files changed, 170 insertions(+), 9 deletions(-) create mode 100644 xrspatial/geotiff/tests/test_signature_parity_1631.py diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index a639637b..903ab36f 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -2663,7 +2663,7 @@ def _read_once(): return result -def write_geotiff_gpu(data, path: str, *, +def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray, path: str, *, crs: int | str | None = None, nodata=None, compression: str = 'zstd', @@ -2726,7 +2726,10 @@ def write_geotiff_gpu(data, path: str, *, halving until the smallest overview fits in a single tile. overview_resampling : str Resampling method for overviews: 'mean' (default), 'nearest', - 'min', 'max', 'median', or 'mode'. + 'min', 'max', 'median', 'mode', or 'cubic'. ``mode`` and + ``cubic`` fall back to the CPU implementation in + ``xrspatial.geotiff._writer`` so the GPU writer produces the + same overview bytes as the CPU writer. bigtiff : bool or None Force BigTIFF (64-bit offsets). None auto-promotes when the estimated file size would exceed the classic-TIFF 4 GB limit. @@ -3187,7 +3190,10 @@ def _sentinel_for_dtype(nodata_val, dtype): return result -def write_vrt(vrt_path: str, source_files: list[str], **kwargs) -> str: +def write_vrt(vrt_path: str, source_files: list[str], *, + relative: bool = True, + crs_wkt: str | None = None, + nodata: float | None = None) -> str: """Generate a VRT file that mosaics multiple GeoTIFF tiles. Parameters @@ -3208,14 +3214,18 @@ def write_vrt(vrt_path: str, source_files: list[str], **kwargs) -> str: ------- str Path to the written VRT file. - - Notes - ----- - Only the keyword arguments listed above are accepted. Passing any - other keyword raises ``TypeError`` from the underlying writer. """ + # Explicit signature (previously ``**kwargs``) so ``inspect.signature``, + # IDE autocomplete, and ``mypy --strict`` can see the accepted kwargs + # without parsing the docstring. Mirrors ``_vrt.write_vrt`` exactly; if + # that signature changes, this wrapper must be updated in lockstep. from ._vrt import write_vrt as _write_vrt_internal - return _write_vrt_internal(vrt_path, source_files, **kwargs) + return _write_vrt_internal( + vrt_path, source_files, + relative=relative, + crs_wkt=crs_wkt, + nodata=nodata, + ) def plot_geotiff(da: xr.DataArray, **kwargs): diff --git a/xrspatial/geotiff/tests/test_signature_parity_1631.py b/xrspatial/geotiff/tests/test_signature_parity_1631.py new file mode 100644 index 00000000..49e0d950 --- /dev/null +++ b/xrspatial/geotiff/tests/test_signature_parity_1631.py @@ -0,0 +1,151 @@ +"""Regression test for #1631: public write_vrt / write_geotiff_gpu +signature and docstring parity vs to_geotiff. + +Three drifts were flagged by the api-consistency sweep on 2026-05-11: + +1. ``write_vrt(vrt_path, source_files, **kwargs)`` swallowed every kwarg + into ``**kwargs``. The docstring documented ``relative``, ``crs_wkt``, + ``nodata``, but ``inspect.signature`` and IDE autocomplete saw nothing. +2. ``write_geotiff_gpu``'s ``overview_resampling`` docstring omitted + ``'cubic'``; ``to_geotiff`` lists it and ``make_overview_gpu`` accepts + it (falling back to CPU). +3. ``write_geotiff_gpu(data, ...)`` lacked the type hint that + ``to_geotiff(data, ...)`` has. + +This module pins each of those three guarantees against future drift. +""" + +import inspect +import os +import tempfile + +import numpy as np +import pytest +import xarray as xr + +from xrspatial.geotiff import ( + open_geotiff, + to_geotiff, + write_geotiff_gpu, + write_vrt, +) + + +def test_write_vrt_signature_exposes_documented_kwargs(): + """``inspect.signature(write_vrt)`` reports the three accepted kwargs. + + Prior to #1631 the public wrapper used ``**kwargs``, so + ``inspect.signature`` only saw ``vrt_path`` and ``source_files``. + """ + sig = inspect.signature(write_vrt) + params = sig.parameters + assert 'relative' in params + assert 'crs_wkt' in params + assert 'nodata' in params + # Defaults must match _vrt.write_vrt + assert params['relative'].default is True + assert params['crs_wkt'].default is None + assert params['nodata'].default is None + # No more catch-all VAR_KEYWORD + kinds = {p.kind for p in params.values()} + assert inspect.Parameter.VAR_KEYWORD not in kinds + + +def test_write_vrt_unknown_kwarg_rejected_at_public_level(): + """A typo'd kwarg now raises ``TypeError`` from the public function + rather than from deep inside ``_vrt.write_vrt``. + """ + with tempfile.TemporaryDirectory() as td: + arr = np.zeros((8, 8), dtype=np.float32) + da = xr.DataArray( + arr, dims=['y', 'x'], + coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, + attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, + ) + tif_path = os.path.join(td, 't.tif') + to_geotiff(da, tif_path) + + with pytest.raises(TypeError, match='typo_kwarg'): + write_vrt(os.path.join(td, 't.vrt'), [tif_path], typo_kwarg=1) + + +def test_write_vrt_accepts_documented_kwargs(): + """Each documented kwarg round-trips through the explicit signature.""" + with tempfile.TemporaryDirectory() as td: + arr = np.zeros((8, 8), dtype=np.float32) + da = xr.DataArray( + arr, dims=['y', 'x'], + coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, + attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, + ) + tif_path = os.path.join(td, 't.tif') + to_geotiff(da, tif_path) + + vrt_path = os.path.join(td, 't.vrt') + out = write_vrt( + vrt_path, [tif_path], + relative=False, crs_wkt=None, nodata=-9999.0, + ) + assert out == vrt_path + assert os.path.exists(vrt_path) + + +def test_write_geotiff_gpu_docstring_lists_cubic(): + """``overview_resampling`` docstring includes ``'cubic'`` so it + matches ``to_geotiff`` and the underlying ``make_overview_gpu``. + """ + doc = write_geotiff_gpu.__doc__ + assert doc is not None + # Find the overview_resampling block + assert 'overview_resampling' in doc + # The block must mention cubic + block_start = doc.index('overview_resampling') + block_end = doc.index('bigtiff', block_start) + block = doc[block_start:block_end] + assert 'cubic' in block + + +def test_write_geotiff_gpu_data_has_type_hint(): + """``data`` parameter is annotated, matching ``to_geotiff(data, ...)``.""" + sig = inspect.signature(write_geotiff_gpu) + data_param = sig.parameters['data'] + assert data_param.annotation is not inspect.Parameter.empty + # The annotation is a forward reference under ``from __future__ import + # annotations``; just confirm it mentions the documented types. + ann_str = str(data_param.annotation) + assert 'DataArray' in ann_str or 'cupy' in ann_str + + +@pytest.mark.skipif( + not pytest.importorskip('cupy', reason='cupy required').is_available() + if False else False, + reason='guarded below', +) +def test_write_geotiff_gpu_cubic_overview_round_trip(): + """``overview_resampling='cubic'`` works on the GPU writer. + + Sanity check that the docstring update is not advertising an + unsupported codec. ``make_overview_gpu`` falls back to the CPU + cubic implementation for parity with the CPU writer. + """ + cupy = pytest.importorskip('cupy') + try: + cupy.zeros(1) + except Exception: + pytest.skip('cupy import succeeded but no device available') + + with tempfile.TemporaryDirectory() as td: + arr_cpu = np.random.RandomState(0).rand(256, 256).astype(np.float32) + arr_gpu = cupy.asarray(arr_cpu) + da_gpu = xr.DataArray( + arr_gpu, dims=['y', 'x'], + coords={'y': np.arange(256.0, 0, -1), 'x': np.arange(256.0)}, + ) + path = os.path.join(td, 'cog.tif') + write_geotiff_gpu( + da_gpu, path, + cog=True, tile_size=64, overview_resampling='cubic', + ) + # Overview level 1 = 1/2 resolution + ov = open_geotiff(path, overview_level=1) + assert ov.shape == (128, 128) From 2b1af43cfb86a3e92ed3a3f5b00889ad2969f71c Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 11 May 2026 14:17:41 -0700 Subject: [PATCH 2/4] Update sweep-api-consistency-state for geotiff (#1631) --- .claude/sweep-api-consistency-state.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/sweep-api-consistency-state.csv b/.claude/sweep-api-consistency-state.csv index 3403bb13..3ff54bea 100644 --- a/.claude/sweep-api-consistency-state.csv +++ b/.claude/sweep-api-consistency-state.csv @@ -1,3 +1,3 @@ module,last_inspected,issue,severity_max,categories_found,notes -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." +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." 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)." From ce88a2c91221d098aa7e03813765456e226fdb45 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 11 May 2026 15:34:38 -0700 Subject: [PATCH 3/4] 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. --- xrspatial/geotiff/__init__.py | 9 ++-- .../tests/test_signature_parity_1631.py | 44 ++++++++++++++----- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index 903ab36f..4ad1f2fd 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -2663,7 +2663,8 @@ def _read_once(): return result -def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray, path: str, *, +def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray, + path: str, *, crs: int | str | None = None, nodata=None, compression: str = 'zstd', @@ -2692,8 +2693,10 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray, path: str, *, Parameters ---------- - data : xr.DataArray (CuPy-backed) or cupy.ndarray - 2D raster on GPU. + data : xr.DataArray (CuPy- or NumPy-backed), cupy.ndarray, or np.ndarray + 2D or 3D raster. CuPy-backed inputs stay on device; NumPy/Dask + inputs are uploaded via ``cupy.asarray(np.asarray(data))`` + before compression (matches ``to_geotiff`` parity). path : str Output file path. crs : int, str, or None diff --git a/xrspatial/geotiff/tests/test_signature_parity_1631.py b/xrspatial/geotiff/tests/test_signature_parity_1631.py index 49e0d950..b3d36ee1 100644 --- a/xrspatial/geotiff/tests/test_signature_parity_1631.py +++ b/xrspatial/geotiff/tests/test_signature_parity_1631.py @@ -14,7 +14,9 @@ This module pins each of those three guarantees against future drift. """ +from __future__ import annotations +import importlib.util import inspect import os import tempfile @@ -31,6 +33,24 @@ ) +def _gpu_available() -> bool: + """True when cupy imports and CUDA is initialised.""" + if importlib.util.find_spec("cupy") is None: + return False + try: + import cupy + + return bool(cupy.cuda.is_available()) + except Exception: + return False + + +_HAS_GPU = _gpu_available() +_gpu_only = pytest.mark.skipif( + not _HAS_GPU, reason="cupy + CUDA required", +) + + def test_write_vrt_signature_exposes_documented_kwargs(): """``inspect.signature(write_vrt)`` reports the three accepted kwargs. @@ -106,21 +126,25 @@ def test_write_geotiff_gpu_docstring_lists_cubic(): def test_write_geotiff_gpu_data_has_type_hint(): - """``data`` parameter is annotated, matching ``to_geotiff(data, ...)``.""" + """``data`` parameter is annotated, matching ``to_geotiff(data, ...)``. + + The annotation also covers ``np.ndarray`` because the implementation + accepts numpy inputs (uploaded via ``cupy.asarray(np.asarray(data))``) + and the test suite exercises that path (e.g. + ``test_backend_kwarg_parity_1561.py`` passes a numpy ``dummy``). + """ sig = inspect.signature(write_geotiff_gpu) data_param = sig.parameters['data'] assert data_param.annotation is not inspect.Parameter.empty # The annotation is a forward reference under ``from __future__ import # annotations``; just confirm it mentions the documented types. ann_str = str(data_param.annotation) - assert 'DataArray' in ann_str or 'cupy' in ann_str + assert 'DataArray' in ann_str + assert 'cupy' in ann_str + assert 'ndarray' in ann_str # numpy parity vs to_geotiff -@pytest.mark.skipif( - not pytest.importorskip('cupy', reason='cupy required').is_available() - if False else False, - reason='guarded below', -) +@_gpu_only def test_write_geotiff_gpu_cubic_overview_round_trip(): """``overview_resampling='cubic'`` works on the GPU writer. @@ -128,11 +152,7 @@ def test_write_geotiff_gpu_cubic_overview_round_trip(): unsupported codec. ``make_overview_gpu`` falls back to the CPU cubic implementation for parity with the CPU writer. """ - cupy = pytest.importorskip('cupy') - try: - cupy.zeros(1) - except Exception: - pytest.skip('cupy import succeeded but no device available') + import cupy with tempfile.TemporaryDirectory() as td: arr_cpu = np.random.RandomState(0).rand(256, 256).astype(np.float32) From cc737e017087ad11b886652ae28c20edb1c77acf Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 11 May 2026 16:52:32 -0700 Subject: [PATCH 4/4] 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. --- .../tests/test_signature_parity_1631.py | 90 +++++++++---------- 1 file changed, 43 insertions(+), 47 deletions(-) diff --git a/xrspatial/geotiff/tests/test_signature_parity_1631.py b/xrspatial/geotiff/tests/test_signature_parity_1631.py index b3d36ee1..2f7e0378 100644 --- a/xrspatial/geotiff/tests/test_signature_parity_1631.py +++ b/xrspatial/geotiff/tests/test_signature_parity_1631.py @@ -19,7 +19,6 @@ import importlib.util import inspect import os -import tempfile import numpy as np import pytest @@ -71,43 +70,41 @@ def test_write_vrt_signature_exposes_documented_kwargs(): assert inspect.Parameter.VAR_KEYWORD not in kinds -def test_write_vrt_unknown_kwarg_rejected_at_public_level(): +def test_write_vrt_unknown_kwarg_rejected_at_public_level(tmp_path): """A typo'd kwarg now raises ``TypeError`` from the public function rather than from deep inside ``_vrt.write_vrt``. """ - with tempfile.TemporaryDirectory() as td: - arr = np.zeros((8, 8), dtype=np.float32) - da = xr.DataArray( - arr, dims=['y', 'x'], - coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, - attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, - ) - tif_path = os.path.join(td, 't.tif') - to_geotiff(da, tif_path) + arr = np.zeros((8, 8), dtype=np.float32) + da = xr.DataArray( + arr, dims=['y', 'x'], + coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, + attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, + ) + tif_path = str(tmp_path / 't.tif') + to_geotiff(da, tif_path) - with pytest.raises(TypeError, match='typo_kwarg'): - write_vrt(os.path.join(td, 't.vrt'), [tif_path], typo_kwarg=1) + with pytest.raises(TypeError, match='typo_kwarg'): + write_vrt(str(tmp_path / 't.vrt'), [tif_path], typo_kwarg=1) -def test_write_vrt_accepts_documented_kwargs(): +def test_write_vrt_accepts_documented_kwargs(tmp_path): """Each documented kwarg round-trips through the explicit signature.""" - with tempfile.TemporaryDirectory() as td: - arr = np.zeros((8, 8), dtype=np.float32) - da = xr.DataArray( - arr, dims=['y', 'x'], - coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, - attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, - ) - tif_path = os.path.join(td, 't.tif') - to_geotiff(da, tif_path) - - vrt_path = os.path.join(td, 't.vrt') - out = write_vrt( - vrt_path, [tif_path], - relative=False, crs_wkt=None, nodata=-9999.0, - ) - assert out == vrt_path - assert os.path.exists(vrt_path) + arr = np.zeros((8, 8), dtype=np.float32) + da = xr.DataArray( + arr, dims=['y', 'x'], + coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, + attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, + ) + tif_path = str(tmp_path / 't.tif') + to_geotiff(da, tif_path) + + vrt_path = str(tmp_path / 't.vrt') + out = write_vrt( + vrt_path, [tif_path], + relative=False, crs_wkt=None, nodata=-9999.0, + ) + assert out == vrt_path + assert os.path.exists(vrt_path) def test_write_geotiff_gpu_docstring_lists_cubic(): @@ -145,7 +142,7 @@ def test_write_geotiff_gpu_data_has_type_hint(): @_gpu_only -def test_write_geotiff_gpu_cubic_overview_round_trip(): +def test_write_geotiff_gpu_cubic_overview_round_trip(tmp_path): """``overview_resampling='cubic'`` works on the GPU writer. Sanity check that the docstring update is not advertising an @@ -154,18 +151,17 @@ def test_write_geotiff_gpu_cubic_overview_round_trip(): """ import cupy - with tempfile.TemporaryDirectory() as td: - arr_cpu = np.random.RandomState(0).rand(256, 256).astype(np.float32) - arr_gpu = cupy.asarray(arr_cpu) - da_gpu = xr.DataArray( - arr_gpu, dims=['y', 'x'], - coords={'y': np.arange(256.0, 0, -1), 'x': np.arange(256.0)}, - ) - path = os.path.join(td, 'cog.tif') - write_geotiff_gpu( - da_gpu, path, - cog=True, tile_size=64, overview_resampling='cubic', - ) - # Overview level 1 = 1/2 resolution - ov = open_geotiff(path, overview_level=1) - assert ov.shape == (128, 128) + arr_cpu = np.random.RandomState(0).rand(256, 256).astype(np.float32) + arr_gpu = cupy.asarray(arr_cpu) + da_gpu = xr.DataArray( + arr_gpu, dims=['y', 'x'], + coords={'y': np.arange(256.0, 0, -1), 'x': np.arange(256.0)}, + ) + path = str(tmp_path / 'cog.tif') + write_geotiff_gpu( + da_gpu, path, + cog=True, tile_size=64, overview_resampling='cubic', + ) + # Overview level 1 = 1/2 resolution + ov = open_geotiff(path, overview_level=1) + assert ov.shape == (128, 128)