Skip to content

geotiff: write_geotiff_gpu kwarg order matches to_geotiff (#1922)#1925

Merged
brendancol merged 1 commit into
mainfrom
deep-sweep-api-consistency-geotiff-2026-05-15-1778854324
May 15, 2026
Merged

geotiff: write_geotiff_gpu kwarg order matches to_geotiff (#1922)#1925
brendancol merged 1 commit into
mainfrom
deep-sweep-api-consistency-geotiff-2026-05-15-1778854324

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

  • 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. Both kwargs are keyword-only so caller code did 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" promise.
  • Reorder write_geotiff_gpu to match to_geotiff (streaming_buffer_bytes before max_z_error). 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. Docstring reordered 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.

Test plan

  • pytest xrspatial/geotiff/tests/test_writer_kwarg_order_1922.py (2 passed)
  • pytest xrspatial/geotiff/tests/test_signature_parity_1631.py xrspatial/geotiff/tests/test_signature_annotations_1654.py xrspatial/geotiff/tests/test_signature_annotations_1705.py (34 passed)
  • pytest xrspatial/geotiff/tests/ -k "max_z_error or streaming_buffer or kwarg_order" (14 passed)
  • pytest xrspatial/geotiff/tests/test_to_geotiff_allow_internal_only_jpeg_parity.py (8 passed)

Copilot AI review requested due to automatic review settings May 15, 2026 14:19
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 15, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns the public GPU GeoTIFF writer signature with to_geotiff so documented API-parity siblings present matching keyword order in introspection, IDEs, and generated docs.

Changes:

  • Reordered write_geotiff_gpu keyword-only parameters so streaming_buffer_bytes precedes max_z_error.
  • Reordered the matching GPU writer docstring parameter entries.
  • Added regression tests for kwarg-order parity and shared default parity.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
xrspatial/geotiff/_writers/gpu.py Aligns write_geotiff_gpu signature and docs with to_geotiff.
xrspatial/geotiff/tests/test_writer_kwarg_order_1922.py Adds regression coverage for signature order and default parity.
.claude/sweep-api-consistency-state.csv Updates sweep state metadata for issue #1922.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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
@brendancol brendancol force-pushed the deep-sweep-api-consistency-geotiff-2026-05-15-1778854324 branch from 8422bc9 to 7c790a3 Compare May 15, 2026 14:38
brendancol added a commit that referenced this pull request May 15, 2026
read_geotiff_gpu and read_geotiff_dask listed shared keyword-only
params in different orders than open_geotiff. The kwargs are all
keyword-only so callers were not broken, but inspect.signature, IDE
autocomplete, and Sphinx docs all showed the drift.

Pick open_geotiff as the canonical reference (the public dispatcher).
Swap window and overview_level in read_geotiff_gpu. Reorder
read_geotiff_dask so window, overview_level, band, name, chunks,
max_pixels match the canonical position. read_vrt already conformed
and is covered by the new regression test.

Regression test (test_reader_kwarg_order_1935.py) pins each reader's
kw-only param order via inspect.signature and asserts no pairwise
inversions across the four readers, so future kwargs cannot be
added in arbitrary positions.

Mirrors the writer-side fix in #1922 / #1925.
@brendancol brendancol merged commit 51419aa into main May 15, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants