Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/sweep-api-consistency-state.csv
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module,last_inspected,issue,severity_max,categories_found,notes
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."
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."
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)."
12 changes: 6 additions & 6 deletions xrspatial/geotiff/_writers/gpu.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
overview_levels: list[int] | None = None,
overview_resampling: str = 'mean',
bigtiff: bool | None = None,
max_z_error: float = 0.0,
streaming_buffer_bytes: int = 256 * 1024 * 1024,
max_z_error: float = 0.0,
photometric: str | int = 'auto',
allow_internal_only_jpeg: bool = False) -> None:
"""Write a CuPy-backed DataArray as a GeoTIFF with GPU compression.
Expand Down Expand Up @@ -146,17 +146,17 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
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.
max_z_error : float
Per-pixel error budget for LERC compression. The GPU writer
does not implement LERC (nvCOMP has no LERC backend), so any
non-zero value raises ``ValueError``. Accepted at the signature
level for API parity with ``to_geotiff``.
streaming_buffer_bytes : int
Accepted for API parity with ``to_geotiff``. The GPU writer
materialises the entire array on device and has no streaming
concept, so this kwarg is a no-op. Default matches
``to_geotiff`` (256 MB) so callers passing the same kwargs to
either entry point see the same default and the same type.
max_z_error : float
Per-pixel error budget for LERC compression. The GPU writer
does not implement LERC (nvCOMP has no LERC backend), so any
non-zero value raises ``ValueError``. Accepted at the signature
level for API parity with ``to_geotiff``.
photometric : str or int
Photometric interpretation for the TIFF Photometric tag (262).
See :func:`to_geotiff` for the full set of accepted values; the
Expand Down
76 changes: 76 additions & 0 deletions xrspatial/geotiff/tests/test_writer_kwarg_order_1922.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
"""Regression test for #1922: write_geotiff_gpu kwarg order matches
to_geotiff (with the documented exception for ``gpu``).

The two writers are advertised as parity siblings. The GPU writer's
own docstring says "Accepted at the signature level for API parity with
``to_geotiff``" for ``max_z_error`` and ``streaming_buffer_bytes``, but
the two kwargs were in opposite order across the two signatures:

to_geotiff: ..., bigtiff, gpu, streaming_buffer_bytes,
max_z_error, photometric, ...
write_geotiff_gpu: ..., bigtiff, max_z_error,
streaming_buffer_bytes, photometric, ...

Both are keyword-only so calling code did not break, but
``inspect.signature()``, IDE autocomplete, and Sphinx-rendered docs all
exposed the drift. Detected by deep-sweep-api-consistency on 2026-05-15.
"""
from __future__ import annotations

import inspect

from xrspatial.geotiff import to_geotiff, write_geotiff_gpu


def test_writer_kwarg_order_matches_to_geotiff():
"""``write_geotiff_gpu`` lists its kwargs in the same order as
``to_geotiff``, modulo the ``gpu`` kwarg the GPU writer omits.

Both signatures use keyword-only kwargs so positional callers are
unaffected. The order still matters for IDE autocomplete, generated
docs, and any caller that inspects ``inspect.signature``.
"""
eager_params = list(inspect.signature(to_geotiff).parameters)
gpu_params = list(inspect.signature(write_geotiff_gpu).parameters)

# to_geotiff has ``gpu`` (auto-dispatch flag); write_geotiff_gpu does
# not. Drop it from the comparison instead of asserting on the
# missing kwarg directly, so unrelated future additions to either
# signature still surface here.
assert 'gpu' in eager_params
assert 'gpu' not in gpu_params
eager_params_no_gpu = [p for p in eager_params if p != 'gpu']

assert gpu_params == eager_params_no_gpu, (
"write_geotiff_gpu and to_geotiff kwarg order diverged.\n"
f" to_geotiff (with 'gpu' removed): {eager_params_no_gpu}\n"
f" write_geotiff_gpu: {gpu_params}\n"
"Reorder write_geotiff_gpu to match to_geotiff (see #1922)."
)


def test_writer_kwarg_defaults_match_to_geotiff():
"""The kwargs both writers share also have identical defaults.

A surprise-free dispatch ``to_geotiff(..., gpu=True)`` requires
``write_geotiff_gpu`` to default the same way for every kwarg the
auto-dispatch entry point forwards (issue #1916 added
``allow_internal_only_jpeg`` to satisfy that contract; this test
pins the broader parity).
"""
eager_sig = inspect.signature(to_geotiff)
gpu_sig = inspect.signature(write_geotiff_gpu)

shared = set(eager_sig.parameters) & set(gpu_sig.parameters)
# ``data`` and ``path`` are required positionals with no default;
# comparing inspect.Parameter.empty against itself is fine.
mismatches = []
for name in sorted(shared):
ed = eager_sig.parameters[name].default
gd = gpu_sig.parameters[name].default
if ed != gd:
mismatches.append((name, ed, gd))
assert not mismatches, (
"write_geotiff_gpu and to_geotiff disagree on defaults: "
f"{mismatches}"
)
Loading