geotiff: to_geotiff accepts allow_internal_only_jpeg for GPU-writer parity#1916
Merged
Merged
Conversation
…arity write_geotiff_gpu gained an allow_internal_only_jpeg opt-in (#1845) so callers who explicitly want the experimental internal-reader-only JPEG-in-TIFF encode path could reach it. to_geotiff kept rejecting compression='jpeg' unconditionally at the top of the function, so the to_geotiff(..., gpu=True) auto-dispatch path could never reach the GPU writer's opt-in: the rejection fired before dispatch. The two public writers therefore disagreed about the supported codec set. Callers reading the GPU writer's docstring saw an opt-in flag they could not actually invoke through the public dispatcher. Add allow_internal_only_jpeg to to_geotiff (default False, matching write_geotiff_gpu), gate the up-front jpeg rejection on it, emit GeoTIFFFallbackWarning when the opt-in fires on the CPU path, and forward the kwarg to write_geotiff_gpu so the GPU dispatch path reaches the same opt-in. Test file pins the signature, the rejection, the opt-in warning, internal-reader round-trip, the non-jpeg no-op, and the shared default. Found by deep-sweep-api-consistency 2026-05-15 sweep.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns to_geotiff with write_geotiff_gpu by exposing the allow_internal_only_jpeg opt-in for experimental JPEG-in-TIFF writes and forwarding it through GPU dispatch.
Changes:
- Adds
allow_internal_only_jpegtoto_geotiff, including validation, warning behavior, and GPU forwarding. - Adds regression tests for signature parity, JPEG rejection, opt-in CPU write behavior, and shared defaults.
- Updates the API consistency sweep state for the geotiff follow-up finding.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xrspatial/geotiff/_writers/eager.py |
Adds the new to_geotiff kwarg, docs, warning path, and GPU writer forwarding. |
xrspatial/geotiff/tests/test_to_geotiff_allow_internal_only_jpeg_parity.py |
Adds tests covering the public dispatcher’s new JPEG opt-in behavior. |
.claude/sweep-api-consistency-state.csv |
Records the resolved API consistency finding for geotiff. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+261
to
+270
| if compression.lower() == 'jpeg' and allow_internal_only_jpeg: | ||
| warnings.warn( | ||
| "to_geotiff(compression='jpeg', " | ||
| "allow_internal_only_jpeg=True) writes JFIF tiles " | ||
| "without the TIFF JPEGTables tag (347); the file decodes " | ||
| "through xrspatial but may fail in libtiff, GDAL, or " | ||
| "rasterio. See issue #1845.", | ||
| GeoTIFFFallbackWarning, | ||
| stacklevel=2, | ||
| ) |
| bigtiff=bigtiff, | ||
| streaming_buffer_bytes=streaming_buffer_bytes, | ||
| photometric=photometric, | ||
| allow_internal_only_jpeg=allow_internal_only_jpeg, |
Comment on lines
+192
to
+200
| allow_internal_only_jpeg : bool | ||
| Opt in to the experimental ``compression='jpeg'`` encode path | ||
| (default ``False``). The encoder writes self-contained JFIF | ||
| tiles without the TIFF JPEGTables tag (347); the file decodes | ||
| through this library's reader but not through libtiff, GDAL, | ||
| or rasterio. With the flag set, the write proceeds and a | ||
| ``GeoTIFFFallbackWarning`` is emitted at call time. Without | ||
| the flag, ``compression='jpeg'`` raises ``ValueError``. The | ||
| kwarg is forwarded unchanged to ``write_geotiff_gpu`` on the |
- Move the JPEG opt-in GeoTIFFFallbackWarning past the GPU dispatch decision so to_geotiff(gpu=True, compression='jpeg', allow_internal_only_jpeg=True) warns once. The CPU dispatcher used to warn before delegating to write_geotiff_gpu, which then emitted its own identical warning -- double-warning callers on the GPU path. The warning now fires only when the call stays on CPU (or routes through VRT, which is CPU-only); the GPU writer continues to emit its own warning on the GPU path. - Update the ``compression`` docstring to mention the allow_internal_only_jpeg opt-in so the public docs no longer claim jpeg is unconditionally rejected on write. - Add two tests covering the GPU dispatch contract: one verifies that allow_internal_only_jpeg is forwarded unchanged into write_geotiff_gpu, the other monkeypatches the GPU writer to emit its own warning and asserts exactly one warning surfaces on the combined gpu=True + opt-in path.
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
write_geotiff_gpugained anallow_internal_only_jpegopt-in in write_geotiff_gpu emits JPEG TIFFs that other readers reject #1845 so callers wanting the experimental internal-reader-only JPEG-in-TIFF encode could reach it.to_geotiffkept rejectingcompression='jpeg'unconditionally at the top of the function, so theto_geotiff(..., gpu=True)auto-dispatch could never reach the GPU writer's opt-in path: the rejection fired before dispatch.allow_internal_only_jpegtoto_geotiff(defaultFalse, matchingwrite_geotiff_gpu), gate the up-front jpeg rejection on it, emitGeoTIFFFallbackWarningwhen the opt-in fires on the CPU path, and forward the kwarg through towrite_geotiff_gpuon the GPU dispatch path.Findings
to_geotiffmissingallow_internal_only_jpegFound by
/deep-sweep(api-consistency sweep against geotiff) on 2026-05-15.Test plan
test_to_geotiff_allow_internal_only_jpeg_parity.py(6 tests pin signature, rejection, opt-in warning + write, non-jpeg no-op, shared default)test_jpeg.py(19 tests) still passtest_gpu_jpeg_interop_reject_issue_D_1845.py(5 tests) still passto_geotiff(da, path, compression='jpeg', allow_internal_only_jpeg=True)writes a file the internal reader decodes; libtiff / GDAL / rasterio still reject the file (the warning is the documented escape hatch)