-
Notifications
You must be signed in to change notification settings - Fork 85
Fix write_vrt and write_geotiff_gpu signature/docstring drift vs to_geotiff (#1631) #1633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
brendancol
merged 4 commits into
xarray-contrib:main
from
brendancol:deep-sweep-api-consistency-geotiff-2026-05-11-1778533682
May 11, 2026
+192
−12
Merged
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
29c215b
Fix write_vrt and write_geotiff_gpu signature/docstring drift vs to_g…
brendancol 2b1af43
Update sweep-api-consistency-state for geotiff (#1631)
brendancol ce88a2c
Address Copilot review on #1633
brendancol cc737e0
Fix Windows tempfile cleanup PermissionError in test_signature_parity
brendancol File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| 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-11,1605,HIGH,1;5,"Filed open_geotiff(gpu=True) silently dropping window/band (HIGH, #1605) and fix PR -- adds window+band kwargs to read_geotiff_gpu and forwards them through open_geotiff GPU dispatch. Prior issues 1560/1561/1562 from 2026-05-10 audit all CLOSED. MEDIUM: read_geotiff_dask VRT defensive fallback drops window/band/max_pixels at line 1463 (acceptable since open_geotiff routes earlier and direct callers can switch to read_vrt). write_vrt wrapper uses **kwargs instead of explicit signature -- docs list the args but inspect.signature does not; cosmetic." | ||
| geotiff,2026-05-11,1631,MEDIUM,3,"Filed write_vrt and write_geotiff_gpu signature/docstring drift vs to_geotiff (MEDIUM, #1631). Fix in PR (TBD): explicit write_vrt(relative, crs_wkt, nodata) signature (was **kwargs); 'cubic' added to write_geotiff_gpu overview_resampling docstring; write_geotiff_gpu(data) typed xr.DataArray|cupy.ndarray to match to_geotiff. Prior 1605/1606/1611/1612/1613/1615/1623 all CLOSED." | ||
| 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)." |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| """Regression test for #1631: public write_vrt / write_geotiff_gpu | ||
| signature and docstring parity vs to_geotiff. | ||
|
|
||
| Three drifts were flagged by the api-consistency sweep on 2026-05-11: | ||
|
|
||
| 1. ``write_vrt(vrt_path, source_files, **kwargs)`` swallowed every kwarg | ||
| into ``**kwargs``. The docstring documented ``relative``, ``crs_wkt``, | ||
| ``nodata``, but ``inspect.signature`` and IDE autocomplete saw nothing. | ||
| 2. ``write_geotiff_gpu``'s ``overview_resampling`` docstring omitted | ||
| ``'cubic'``; ``to_geotiff`` lists it and ``make_overview_gpu`` accepts | ||
| it (falling back to CPU). | ||
| 3. ``write_geotiff_gpu(data, ...)`` lacked the type hint that | ||
| ``to_geotiff(data, ...)`` has. | ||
|
|
||
| This module pins each of those three guarantees against future drift. | ||
| """ | ||
|
|
||
| import inspect | ||
| import os | ||
| import tempfile | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
| import xarray as xr | ||
|
|
||
| from xrspatial.geotiff import ( | ||
| open_geotiff, | ||
| to_geotiff, | ||
| write_geotiff_gpu, | ||
| write_vrt, | ||
| ) | ||
|
|
||
|
|
||
| def test_write_vrt_signature_exposes_documented_kwargs(): | ||
| """``inspect.signature(write_vrt)`` reports the three accepted kwargs. | ||
|
|
||
| Prior to #1631 the public wrapper used ``**kwargs``, so | ||
| ``inspect.signature`` only saw ``vrt_path`` and ``source_files``. | ||
| """ | ||
| sig = inspect.signature(write_vrt) | ||
| params = sig.parameters | ||
| assert 'relative' in params | ||
| assert 'crs_wkt' in params | ||
| assert 'nodata' in params | ||
| # Defaults must match _vrt.write_vrt | ||
| assert params['relative'].default is True | ||
| assert params['crs_wkt'].default is None | ||
| assert params['nodata'].default is None | ||
| # No more catch-all VAR_KEYWORD | ||
| kinds = {p.kind for p in params.values()} | ||
| assert inspect.Parameter.VAR_KEYWORD not in kinds | ||
|
|
||
|
|
||
| def test_write_vrt_unknown_kwarg_rejected_at_public_level(): | ||
| """A typo'd kwarg now raises ``TypeError`` from the public function | ||
| rather than from deep inside ``_vrt.write_vrt``. | ||
| """ | ||
| with tempfile.TemporaryDirectory() as td: | ||
| arr = np.zeros((8, 8), dtype=np.float32) | ||
| da = xr.DataArray( | ||
| arr, dims=['y', 'x'], | ||
| coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, | ||
| attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, | ||
| ) | ||
| tif_path = os.path.join(td, 't.tif') | ||
| to_geotiff(da, tif_path) | ||
|
|
||
| with pytest.raises(TypeError, match='typo_kwarg'): | ||
| write_vrt(os.path.join(td, 't.vrt'), [tif_path], typo_kwarg=1) | ||
|
|
||
|
|
||
| def test_write_vrt_accepts_documented_kwargs(): | ||
| """Each documented kwarg round-trips through the explicit signature.""" | ||
| with tempfile.TemporaryDirectory() as td: | ||
| arr = np.zeros((8, 8), dtype=np.float32) | ||
| da = xr.DataArray( | ||
| arr, dims=['y', 'x'], | ||
| coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, | ||
| attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, | ||
| ) | ||
| tif_path = os.path.join(td, 't.tif') | ||
| to_geotiff(da, tif_path) | ||
|
|
||
| vrt_path = os.path.join(td, 't.vrt') | ||
| out = write_vrt( | ||
| vrt_path, [tif_path], | ||
| relative=False, crs_wkt=None, nodata=-9999.0, | ||
| ) | ||
| assert out == vrt_path | ||
| assert os.path.exists(vrt_path) | ||
|
|
||
|
|
||
| def test_write_geotiff_gpu_docstring_lists_cubic(): | ||
| """``overview_resampling`` docstring includes ``'cubic'`` so it | ||
| matches ``to_geotiff`` and the underlying ``make_overview_gpu``. | ||
| """ | ||
| doc = write_geotiff_gpu.__doc__ | ||
| assert doc is not None | ||
| # Find the overview_resampling block | ||
| assert 'overview_resampling' in doc | ||
| # The block must mention cubic | ||
| block_start = doc.index('overview_resampling') | ||
| block_end = doc.index('bigtiff', block_start) | ||
| block = doc[block_start:block_end] | ||
| assert 'cubic' in block | ||
|
|
||
|
|
||
| def test_write_geotiff_gpu_data_has_type_hint(): | ||
| """``data`` parameter is annotated, matching ``to_geotiff(data, ...)``.""" | ||
| sig = inspect.signature(write_geotiff_gpu) | ||
| data_param = sig.parameters['data'] | ||
| assert data_param.annotation is not inspect.Parameter.empty | ||
| # The annotation is a forward reference under ``from __future__ import | ||
| # annotations``; just confirm it mentions the documented types. | ||
| ann_str = str(data_param.annotation) | ||
| assert 'DataArray' in ann_str or 'cupy' in ann_str | ||
|
|
||
|
|
||
| @pytest.mark.skipif( | ||
| not pytest.importorskip('cupy', reason='cupy required').is_available() | ||
| if False else False, | ||
| reason='guarded below', | ||
| ) | ||
|
brendancol marked this conversation as resolved.
Outdated
|
||
| def test_write_geotiff_gpu_cubic_overview_round_trip(): | ||
| """``overview_resampling='cubic'`` works on the GPU writer. | ||
|
|
||
| Sanity check that the docstring update is not advertising an | ||
| unsupported codec. ``make_overview_gpu`` falls back to the CPU | ||
| cubic implementation for parity with the CPU writer. | ||
| """ | ||
| cupy = pytest.importorskip('cupy') | ||
| try: | ||
| cupy.zeros(1) | ||
| except Exception: | ||
| pytest.skip('cupy import succeeded but no device available') | ||
|
|
||
| with tempfile.TemporaryDirectory() as td: | ||
| arr_cpu = np.random.RandomState(0).rand(256, 256).astype(np.float32) | ||
| arr_gpu = cupy.asarray(arr_cpu) | ||
| da_gpu = xr.DataArray( | ||
| arr_gpu, dims=['y', 'x'], | ||
| coords={'y': np.arange(256.0, 0, -1), 'x': np.arange(256.0)}, | ||
| ) | ||
| path = os.path.join(td, 'cog.tif') | ||
| write_geotiff_gpu( | ||
| da_gpu, path, | ||
| cog=True, tile_size=64, overview_resampling='cubic', | ||
| ) | ||
| # Overview level 1 = 1/2 resolution | ||
| ov = open_geotiff(path, overview_level=1) | ||
| assert ov.shape == (128, 128) | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.