geotiff: reject write_geotiff_gpu jpeg by default, add opt-in flag (#1845)#1849
Merged
Conversation
… flag (xarray-contrib#1845) The CPU writer to_geotiff already raised ValueError for compression='jpeg' because the encoder skips the TIFF JPEGTables tag (347) and the resulting files are unreadable by libtiff, GDAL, and rasterio. write_geotiff_gpu sat in the same module and silently accepted the same kwarg, producing the same broken format. Two public entry points in the same module disagreed about the same codec. Reject compression='jpeg' on write_geotiff_gpu by default with the same wording to_geotiff uses. Add allow_internal_only_jpeg=False to keep the experimental internal-reader-only path reachable for callers who want it; when the flag is set the writer proceeds and emits a GeoTIFFFallbackWarning so the round-trip caveat is loud at call time. Updates the four existing GPU JPEG tests to pass the opt-in flag and adds a new test module covering the rejection (CPU-only, no CUDA needed) and the opt-in warning behaviour.
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns write_geotiff_gpu with to_geotiff by rejecting compression='jpeg' by default (both writers produce TIFFs missing the JPEGTables tag and unreadable by libtiff/GDAL/rasterio), while adding an opt-in allow_internal_only_jpeg flag that proceeds with the internal-only encode and emits a GeoTIFFFallbackWarning.
Changes:
- Add
allow_internal_only_jpeg: bool = Falseparameter towrite_geotiff_gpuwith rejection + warning logic. - Update docstring to document the rejection, opt-in, and interop caveat.
- Add new test module for rejection/opt-in behavior and update existing GPU JPEG tests to pass the flag.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| xrspatial/geotiff/init.py | Adds opt-in flag, rejection ValueError, and fallback warning for JPEG path; docstring updated. |
| xrspatial/geotiff/tests/test_gpu_jpeg_interop_reject_issue_D_1845.py | New tests covering default rejection, message parity, case insensitivity, opt-in warning, and non-JPEG no-op. |
| xrspatial/geotiff/tests/test_gpu_writer_compression_modes_2026_05_11.py | Updates existing GPU JPEG tests to set the opt-in flag and assert the warning. |
💡 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.
Closes #1845.
Summary
to_geotiffrejectscompression='jpeg'because the encoder skips the TIFF JPEGTables tag (347); files produced that way fail in libtiff, GDAL, and rasterio.write_geotiff_gpusat in the same module and silently accepted the same kwarg, producing the same broken format.compression='jpeg'onwrite_geotiff_gpuby default with the same wording.allow_internal_only_jpeg: bool = Falseso callers who want the experimental internal-reader-only path can opt in. When the flag is set, the write proceeds and emits aGeoTIFFFallbackWarningat call time.Migration
Callers currently relying on
write_geotiff_gpu(..., compression='jpeg')get aValueErrorafter this PR. Two options:'deflate','zstd','lzw'.allow_internal_only_jpeg=Trueto keep the existing behaviour. The file will still not round-trip through GDAL or rasterio; the warning is the contract.Test plan
test_gpu_jpeg_interop_reject_issue_D_1845.py: 3 CPU-only rejection tests plus 2 GPU-gated opt-in tests, all pass.test_gpu_writer_compression_modes_2026_05_11.pyupdated to pass the opt-in flag; all pass.test_signature_parity_1631,test_signature_annotations_*,test_backend_kwarg_parity_1561): 46 passed.xrspatial/geotiff/tests/run: pre-existing failures only (matplotlibRecursionErrorintest_features, predictor2 big-endian GPU tests, one tile-size GPU test) — all reproduce onorigin/main.