Repository navigation
geotiff: reader kwarg order matches open_geotiff (#1935) - #1936
Merged
brendancol merged 1 commit intoMay 15, 2026
Merged
Conversation
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.
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns the keyword-only parameter order of read_geotiff_gpu and read_geotiff_dask with the canonical open_geotiff signature to eliminate drift visible in inspect.signature, IDE autocomplete, and Sphinx docs. Adds a regression test pinning the order across all four reader entry points.
Changes:
- Swap
window/overview_levelinread_geotiff_gpu. - Reorder
read_geotiff_daskkwargs towindow, overview_level, band, name, chunks, max_pixels. - Add
test_reader_kwarg_order_1935.pyto enforce canonical order and check no pairwise inversions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| xrspatial/geotiff/_backends/gpu.py | Swap order of overview_level and window kwargs to match canonical. |
| xrspatial/geotiff/_backends/dask.py | Reorder kwargs (window, overview_level, band, name, chunks, max_pixels) to match canonical. |
| xrspatial/geotiff/tests/test_reader_kwarg_order_1935.py | New regression test pinning reader kwarg order via inspect.signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
read_geotiff_gpuandread_geotiff_dasklisted shared keyword-only params in different orders thanopen_geotiff. All kwargs are keyword-only so callers were not broken positionally, butinspect.signature, IDE autocomplete, and Sphinx-rendered docs all surfaced the drift.open_geotiffis the canonical reference. Swapwindowandoverview_levelinread_geotiff_gpu. Reorderread_geotiff_dasksowindow,overview_level,band,name,chunks,max_pixelsline up with the canonical position.read_vrtalready conformed.inspect.signatureand asserts no pairwise inversions across the four entry points.Mirrors the writer-side fix in #1922 / #1925.
Closes #1935.
Test plan
pytest xrspatial/geotiff/tests/test_reader_kwarg_order_1935.py -x -q(5 passed)pytest xrspatial/geotiff/tests/test_backend_kwarg_parity_1561.py xrspatial/geotiff/tests/test_signature_parity_1631.py xrspatial/geotiff/tests/test_signature_annotations_1654.py xrspatial/geotiff/tests/test_signature_annotations_1705.py xrspatial/geotiff/tests/test_kwarg_coverage_2026_05_11_r4.py xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12.py xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12_v2.py -x -q(107 passed)