diff --git a/.claude/sweep-api-consistency-state.csv b/.claude/sweep-api-consistency-state.csv index 6b5d4f921..85ee7521e 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-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)." diff --git a/xrspatial/geotiff/_writers/gpu.py b/xrspatial/geotiff/_writers/gpu.py index 155253fcf..5617c1d24 100644 --- a/xrspatial/geotiff/_writers/gpu.py +++ b/xrspatial/geotiff/_writers/gpu.py @@ -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. @@ -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 diff --git a/xrspatial/geotiff/tests/test_writer_kwarg_order_1922.py b/xrspatial/geotiff/tests/test_writer_kwarg_order_1922.py new file mode 100644 index 000000000..1da66d3dd --- /dev/null +++ b/xrspatial/geotiff/tests/test_writer_kwarg_order_1922.py @@ -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}" + )