Block write_geotiff_gpu(file_like, cog=True) for parity with to_geotiff (#1652)#1653
Merged
brendancol merged 2 commits intoMay 12, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns the explicit GPU GeoTIFF writer (write_geotiff_gpu) with to_geotiff behavior by rejecting cog=True when the destination is file-like (e.g., io.BytesIO), and adds documentation + regression tests to prevent API drift.
Changes:
- Add a
cog=True+ file-like guard towrite_geotiff_gpu(mirroringto_geotiff’s restriction). - Expand
write_geotiff_gpudocstring to document file-like support and clarify the deliberate JPEG asymmetry vsto_geotiff. - Add regression tests covering the new rejection and the still-supported non-COG file-like path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xrspatial/geotiff/tests/test_bytesio_source.py |
Adds GPU-writer BytesIO regression tests for cog=True rejection + non-COG BytesIO round-trip. |
xrspatial/geotiff/__init__.py |
Documents file-like destinations for write_geotiff_gpu and adds the cog=True + file-like ValueError gate. |
.claude/sweep-api-consistency-state.csv |
Updates sweep tracking state to record the issue and planned fix coverage. |
Comments suppressed due to low confidence (1)
xrspatial/geotiff/init.py:2721
write_geotiff_gpudrops thepath: strtype annotation but the docstring now advertisesstr or binary file-like. It would be clearer (and more consistent with the rest of the typed signature) to annotatepathas a union (e.g.,str | os.PathLike[str] | BinaryIO/SupportsWrite[bytes]) instead of leaving it untyped.
def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
path, *,
crs: int | str | None = None,
nodata=None,
compression: str = 'zstd',
compression_level: int | None = None,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+284
to
+303
| def test_cog_with_bytesio_rejected_1652(self): | ||
| cupy = pytest.importorskip("cupy") | ||
| da = xr.DataArray( | ||
| cupy.asarray(np.random.rand(64, 64).astype(np.float32)), | ||
| dims=['y', 'x'], | ||
| coords={'y': np.arange(64.0), 'x': np.arange(64.0)}, | ||
| attrs={'crs': 4326}, | ||
| ) | ||
| from xrspatial.geotiff import write_geotiff_gpu | ||
|
|
||
| buf = io.BytesIO() | ||
| with pytest.raises(ValueError, match='cog=True'): | ||
| write_geotiff_gpu(da, buf, cog=True) | ||
|
|
||
| def test_non_cog_bytesio_still_works_1652(self): | ||
| cupy = pytest.importorskip("cupy") | ||
| arr_cpu = np.random.rand(64, 64).astype(np.float32) | ||
| da = xr.DataArray( | ||
| cupy.asarray(arr_cpu), | ||
| dims=['y', 'x'], |
Comment on lines
+2834
to
+2835
| "cog=True is not supported for file-like destinations on the " | ||
| "GPU writer. Pass a string path or set cog=False.") |
Comment on lines
+2825
to
+2831
| # Mirror to_geotiff's file-like + cog=True rejection. The auto-dispatch | ||
| # path through ``to_geotiff(gpu=True, cog=True, path=BytesIO)`` raises | ||
| # before reaching here; the explicit GPU writer mirrors the gate so | ||
| # callers cannot bypass it (issue #1652). Non-cog file-like writes | ||
| # remain supported on this entry point. | ||
| _path_is_file_like = ( | ||
| not isinstance(path, str)) and hasattr(path, 'write') |
brendancol
added a commit
that referenced
this pull request
May 12, 2026
- Mirror to_geotiff's cog=True + file-like rejection verbatim (same
error string), so callers see identical messages from either entry
point. Previously write_geotiff_gpu raised a different message that
added "...on the GPU writer..." and dropped the BytesIO hint.
- Add a TypeError gate for non-str, non-file-like path arguments, so
passing e.g. an int falls through to a clear TypeError instead of an
os.path / unicode error deep in the writer. Mirrors to_geotiff's
existing TypeError verbatim.
- Harden the BytesIO + write_geotiff_gpu tests with the repo's standard
_gpu_available() helper (cupy.cuda.is_available() + ImportError
guard) instead of pytest.importorskip('cupy'), so CI hosts where
CuPy imports but CUDA is unavailable skip cleanly rather than
hard-failing in cupy.asarray().
- Add two new regression tests: one pinning byte-for-byte parity of the
cog/file-like error message between the two writers, and one pinning
the new TypeError on invalid path types.
Skip: the low-confidence type annotation suggestion (path:
str|os.PathLike|SupportsWrite[bytes]). The other path-accepting
functions in this module (to_geotiff, write_vrt, etc.) deliberately
leave path untyped; adding a precise union here would diverge from the
local convention for marginal benefit.
All 17 tests in test_bytesio_source.py pass.
…ff (#1652) to_geotiff has long rejected cog=True + file-like destinations, but the explicit write_geotiff_gpu entry point silently accepted the combo and emitted a COG into the buffer. The two writers should agree on which inputs they refuse: to_geotiff(gpu=True, cog=True, path=BytesIO) raises, so write_geotiff_gpu(da, BytesIO, cog=True) should too. Mirror the existing to_geotiff guard on the GPU entry point. Non-cog file-like writes remain supported on this path (the gate is targeted at cog=True only). Add regression coverage in test_bytesio_source.py. Also clarify the path/compression docstring on write_geotiff_gpu: - path: document that file-like destinations are accepted (cog=True requires a string path). - compression: list the full codec set the function actually accepts and note the deliberate JPEG asymmetry with to_geotiff (#1651 downgraded to docs-only after PR #1647 confirmed advanced-API intent). Update .claude/sweep-api-consistency-state.csv with the 2026-05-11 re-audit row.
- Mirror to_geotiff's cog=True + file-like rejection verbatim (same
error string), so callers see identical messages from either entry
point. Previously write_geotiff_gpu raised a different message that
added "...on the GPU writer..." and dropped the BytesIO hint.
- Add a TypeError gate for non-str, non-file-like path arguments, so
passing e.g. an int falls through to a clear TypeError instead of an
os.path / unicode error deep in the writer. Mirrors to_geotiff's
existing TypeError verbatim.
- Harden the BytesIO + write_geotiff_gpu tests with the repo's standard
_gpu_available() helper (cupy.cuda.is_available() + ImportError
guard) instead of pytest.importorskip('cupy'), so CI hosts where
CuPy imports but CUDA is unavailable skip cleanly rather than
hard-failing in cupy.asarray().
- Add two new regression tests: one pinning byte-for-byte parity of the
cog/file-like error message between the two writers, and one pinning
the new TypeError on invalid path types.
Skip: the low-confidence type annotation suggestion (path:
str|os.PathLike|SupportsWrite[bytes]). The other path-accepting
functions in this module (to_geotiff, write_vrt, etc.) deliberately
leave path untyped; adding a precise union here would diverge from the
local convention for marginal benefit.
All 17 tests in test_bytesio_source.py pass.
brendancol
force-pushed
the
deep-sweep-api-consistency-geotiff-2026-05-11-1778547162
branch
from
9e54f2f to
d91d78c
Compare
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
to_geotiff'scog=True+ file-like guard onwrite_geotiff_gpuso the auto-dispatch and explicit GPU entry points fail the same way (geotiff: write_geotiff_gpu accepts file-like+cog where to_geotiff rejects #1652).to_geotiffin thewrite_geotiff_gpudocstring.TestWriteGeotiffGpuBytesIOtotest_bytesio_source.pycovering the new rejection and the still-supported non-cog file-like path.Background
The deep-sweep api-consistency pass on 2026-05-11 found that:
to_geotiff(da, BytesIO, gpu=True, cog=True)raisedValueError("cog=True is not supported for file-like destinations")(existing guard).write_geotiff_gpu(da, BytesIO, cog=True)silently produced a COG-shaped TIFF inside the buffer.write_geotiff_gpuis documented as "whatto_geotiff(..., gpu=True)calls internally." Users who followto_geotiff's rejection message and switch to the explicit GPU writer should not silently get the resultto_geotiffblocked. Both entry points now raise the sameValueError.A second finding (JPEG acceptance drift, originally HIGH) was downgraded after #1647 confirmed
write_geotiff_gpu(compression='jpeg')is deliberate advanced-API access to the nvJPEG / Pillow encoder. The docstring clarification documents that split.Test plan
pytest xrspatial/geotiff/tests/test_bytesio_source.py-> 15 passed locally (2 new).pytest xrspatial/geotiff/tests/test_gpu_writer_compression_modes_2026_05_11.py-> 12 passed (JPEG path unchanged).test_features.py) -> 1320 passed, 3 skipped on a CUDA host.Closes #1652.