Skip to content

Commit 8422bc9

Browse files
committed
geotiff: write_geotiff_gpu kwarg order matches to_geotiff (#1922)
to_geotiff and write_geotiff_gpu are documented parity siblings, but the two signatures put max_z_error and streaming_buffer_bytes in opposite order: to_geotiff: ..., bigtiff, gpu, streaming_buffer_bytes, max_z_error, photometric, ... write_geotiff_gpu: ..., bigtiff, max_z_error, streaming_buffer_bytes, photometric, ... Both kwargs are keyword-only so caller code does not break, but the drift surfaced in inspect.signature, IDE autocomplete, and Sphinx docs against the GPU writer's own "Accepted at the signature level for API parity with to_geotiff" docstring promise. Reorder write_geotiff_gpu to match to_geotiff. The 'gpu' auto-dispatch kwarg is the only one to_geotiff has that the GPU entry point does not, so the gap stays in place; everything else after 'bigtiff' lines up. Also reorder the docstring to keep doc order parallel. New regression test test_writer_kwarg_order_1922.py pins kwarg order parity (modulo 'gpu') and shared-default parity so future signature edits trip the test instead of silently re-introducing drift. Detected by deep-sweep-api-consistency on 2026-05-15. Refs #1922
1 parent b176169 commit 8422bc9

3 files changed

Lines changed: 83 additions & 7 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-15,1845-followup,HIGH,5,"Sweep 2026-05-15 (deep-sweep-api-consistency-geotiff-2026-05-15). 1 HIGH Cat 5 finding fixed in this branch: to_geotiff was missing allow_internal_only_jpeg, the opt-in flag added to write_geotiff_gpu in #1845. to_geotiff(compression='jpeg', gpu=True, allow_internal_only_jpeg=True) could not reach the GPU writer's opt-in because to_geotiff rejected jpeg up front. Fix mirrors the GPU writer: accept the kwarg with default False, gate the up-front jpeg rejection on it, emit GeoTIFFFallbackWarning on opt-in, forward to write_geotiff_gpu. Regression test in test_to_geotiff_allow_internal_only_jpeg_parity.py (6 tests). Prior findings (#1654 #1683 #1684 #1685 #1705 #1715 #1754 #1775 #1810) all confirmed fixed. Cross-sibling return-type drift (Cat 2): write_vrt returns str while to_geotiff and write_geotiff_gpu return None -- still deferred (LOW, callers do not substitute these writers). cuda-validated."
2+
geotiff,2026-05-15,1922,MEDIUM,1,"Sweep 2026-05-15 (deep-sweep-api-consistency-geotiff-2026-05-15-1778854324). 1 MEDIUM Cat 1 finding fixed in this branch: write_geotiff_gpu and to_geotiff disagreed on order of max_z_error / streaming_buffer_bytes kwargs. Both kwargs are keyword-only so no functional break; drift surfaced in inspect.signature, IDE autocomplete, and Sphinx docs against the writers' explicit-parity promise. Fix reorders write_geotiff_gpu to match to_geotiff (streaming_buffer_bytes before max_z_error) and updates the docstring; gpu is the only kwarg to_geotiff has that write_geotiff_gpu does not, so the gap stays. Regression test in test_writer_kwarg_order_1922.py pins kwarg order parity and default-value parity. Prior findings (#1654 #1683 #1684 #1685 #1705 #1715 #1754 #1775 #1810 #1845-followup) all confirmed fixed. Cross-sibling return-type drift (Cat 2): write_vrt returns str while to_geotiff and write_geotiff_gpu return None -- still deferred (LOW, callers do not substitute these writers). Cross-cutting cross-module drift (chunk_size in reproject vs chunks in geotiff; target_crs vs crs) documented but not filed per sweep template (cross-cutting). cuda-validated."
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/_writers/gpu.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
4343
overview_levels: list[int] | None = None,
4444
overview_resampling: str = 'mean',
4545
bigtiff: bool | None = None,
46-
max_z_error: float = 0.0,
4746
streaming_buffer_bytes: int = 256 * 1024 * 1024,
47+
max_z_error: float = 0.0,
4848
photometric: str | int = 'auto',
4949
allow_internal_only_jpeg: bool = False) -> None:
5050
"""Write a CuPy-backed DataArray as a GeoTIFF with GPU compression.
@@ -146,17 +146,17 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
146146
bigtiff : bool or None
147147
Force BigTIFF (64-bit offsets). None auto-promotes when the
148148
estimated file size would exceed the classic-TIFF 4 GB limit.
149-
max_z_error : float
150-
Per-pixel error budget for LERC compression. The GPU writer
151-
does not implement LERC (nvCOMP has no LERC backend), so any
152-
non-zero value raises ``ValueError``. Accepted at the signature
153-
level for API parity with ``to_geotiff``.
154149
streaming_buffer_bytes : int
155150
Accepted for API parity with ``to_geotiff``. The GPU writer
156151
materialises the entire array on device and has no streaming
157152
concept, so this kwarg is a no-op. Default matches
158153
``to_geotiff`` (256 MB) so callers passing the same kwargs to
159154
either entry point see the same default and the same type.
155+
max_z_error : float
156+
Per-pixel error budget for LERC compression. The GPU writer
157+
does not implement LERC (nvCOMP has no LERC backend), so any
158+
non-zero value raises ``ValueError``. Accepted at the signature
159+
level for API parity with ``to_geotiff``.
160160
photometric : str or int
161161
Photometric interpretation for the TIFF Photometric tag (262).
162162
See :func:`to_geotiff` for the full set of accepted values; the
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
"""Regression test for #1922: write_geotiff_gpu kwarg order matches
2+
to_geotiff (with the documented exception for ``gpu``).
3+
4+
The two writers are advertised as parity siblings. The GPU writer's
5+
own docstring says "Accepted at the signature level for API parity with
6+
``to_geotiff``" for ``max_z_error`` and ``streaming_buffer_bytes``, but
7+
the two kwargs were in opposite order across the two signatures:
8+
9+
to_geotiff: ..., bigtiff, gpu, streaming_buffer_bytes,
10+
max_z_error, photometric, ...
11+
write_geotiff_gpu: ..., bigtiff, max_z_error,
12+
streaming_buffer_bytes, photometric, ...
13+
14+
Both are keyword-only so calling code did not break, but
15+
``inspect.signature()``, IDE autocomplete, and Sphinx-rendered docs all
16+
exposed the drift. Detected by deep-sweep-api-consistency on 2026-05-15.
17+
"""
18+
from __future__ import annotations
19+
20+
import inspect
21+
22+
from xrspatial.geotiff import to_geotiff, write_geotiff_gpu
23+
24+
25+
def test_writer_kwarg_order_matches_to_geotiff():
26+
"""``write_geotiff_gpu`` lists its kwargs in the same order as
27+
``to_geotiff``, modulo the ``gpu`` kwarg the GPU writer omits.
28+
29+
Both signatures use keyword-only kwargs so positional callers are
30+
unaffected. The order still matters for IDE autocomplete, generated
31+
docs, and any caller that inspects ``inspect.signature``.
32+
"""
33+
eager_params = list(inspect.signature(to_geotiff).parameters)
34+
gpu_params = list(inspect.signature(write_geotiff_gpu).parameters)
35+
36+
# to_geotiff has ``gpu`` (auto-dispatch flag); write_geotiff_gpu does
37+
# not. Drop it from the comparison instead of asserting on the
38+
# missing kwarg directly, so unrelated future additions to either
39+
# signature still surface here.
40+
assert 'gpu' in eager_params
41+
assert 'gpu' not in gpu_params
42+
eager_params_no_gpu = [p for p in eager_params if p != 'gpu']
43+
44+
assert gpu_params == eager_params_no_gpu, (
45+
"write_geotiff_gpu and to_geotiff kwarg order diverged.\n"
46+
f" to_geotiff (with 'gpu' removed): {eager_params_no_gpu}\n"
47+
f" write_geotiff_gpu: {gpu_params}\n"
48+
"Reorder write_geotiff_gpu to match to_geotiff (see #1922)."
49+
)
50+
51+
52+
def test_writer_kwarg_defaults_match_to_geotiff():
53+
"""The kwargs both writers share also have identical defaults.
54+
55+
A surprise-free dispatch ``to_geotiff(..., gpu=True)`` requires
56+
``write_geotiff_gpu`` to default the same way for every kwarg the
57+
auto-dispatch entry point forwards (issue #1916 added
58+
``allow_internal_only_jpeg`` to satisfy that contract; this test
59+
pins the broader parity).
60+
"""
61+
eager_sig = inspect.signature(to_geotiff)
62+
gpu_sig = inspect.signature(write_geotiff_gpu)
63+
64+
shared = set(eager_sig.parameters) & set(gpu_sig.parameters)
65+
# ``data`` and ``path`` are required positionals with no default;
66+
# comparing inspect.Parameter.empty against itself is fine.
67+
mismatches = []
68+
for name in sorted(shared):
69+
ed = eager_sig.parameters[name].default
70+
gd = gpu_sig.parameters[name].default
71+
if ed != gd:
72+
mismatches.append((name, ed, gd))
73+
assert not mismatches, (
74+
"write_geotiff_gpu and to_geotiff disagree on defaults: "
75+
f"{mismatches}"
76+
)

0 commit comments

Comments
 (0)