geotiff: cover gil_friendly kwarg behaviour added in #1826 (#1830)#1834
Merged
brendancol merged 3 commits intoMay 14, 2026
Merged
Conversation
PR #1826 added a ``gil_friendly`` flag to ``deflate_compress`` / ``compress`` / ``_prepare_strip`` / ``_prepare_tile`` / ``_compress_block``. The flag forces deflate through stdlib ``zlib.compress`` (GIL-releasing) when set, so the writer's parallel strip/tile paths actually scale across 8 threads (~5x vs ~1.2x with the GIL-holding libdeflate binding). Existing tests covered round-trip correctness and that the thread pool dispatched, but nothing observed which deflate backend ran, which strip/tile branch passed which flag value, or that the one-shot fallback ``UserWarning`` fired when the optional ``deflate`` package was missing. 20 new tests pin every layer of the new contract: - ``deflate_compress(gil_friendly=True)`` bypasses libdeflate; the default keeps using libdeflate when installed; both directions round-trip through stdlib ``zlib.decompress`` at levels 1/6/9. - The fallback UserWarning fires exactly once when libdeflate is missing, stays suppressed when the latch is set, and reuses the latch across subsequent calls (no per-call spam). - ``compress(DEFLATE, gil_friendly=...)`` forwards the flag to the codec; the flag is a no-op for LZW/PackBits/zstd/lz4/none. - ``_write_stripped`` parallel path passes ``gil_friendly=True`` on every strip; the sequential path leaves it at the default ``False``. - ``_write_tiled`` parallel path passes ``True`` positionally; the sequential path leaves it at ``False``. ``_prepare_strip`` and ``_prepare_tile`` forward the kwarg through to ``deflate_compress``. - End-to-end round-trip across both parallelism modes (strip/tile, small/large). Mutation checks confirm coverage: dropping the ``and not gil_friendly`` guard in ``_compression.py`` flips two tests red; flipping the writer's ``gil_friendly=True`` to ``False`` flips the parallel-path test red; deleting the warning flips the warning-fires test red. Closes #1830
The macOS 3.12 CI runner ships without the optional ``lz4`` package, so the unconditional LZ4 row in ``test_compress_gil_friendly_ignored_for_non _deflate_codecs`` raised ImportError before reaching the gil_friendly assertion. Skip the LZ4 row when the codec is unavailable; the rest of the non-deflate codecs (none, packbits, lzw, zstd) still cover the parity check.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds direct GeoTIFF tests for the gil_friendly compression flag introduced in PR #1826, focusing on deflate backend selection and writer forwarding behavior.
Changes:
- Adds tests for
deflate_compressandcompressbackend routing withgil_friendly=True/False. - Adds tests for stripped/tiled writer paths forwarding the flag correctly.
- Adds end-to-end deflate round-trip coverage across sequential and parallel strip/tile modes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+324
to
+325
| # Writer call-site verification: _write_stripped / _write_tiled / | ||
| # write_streaming pass the right gil_friendly value into the codec. |
…1834) Copilot review on PR #1834 flagged that the writer-coverage section header claimed to verify write_streaming but the file only walked _write_stripped / _write_tiled / _prepare_strip / _prepare_tile. The streaming dask writer routes per-tile compression through _compress_block, which carries its own gil_friendly forwarding (True on the parallel tile-segment branch, default False on the serial branch). Add three direct pins: - _compress_block(gil_friendly=True) forwards to deflate_compress with the flag set. - _compress_block default keeps gil_friendly=False so serial segments stay on libdeflate. - write_streaming on a multi-tile dask array drives at least one deflate_compress call with gil_friendly=True, pinning the parallel tile-segment branch end-to-end.
Contributor
Author
|
Addressed the Copilot coverage gap in commit a880bc3. The section header claimed Added three direct pins:
|
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
gil_friendlykwarg PR geotiff: fix libdeflate import, hybrid codec for parallel writer #1826 threaded throughdeflate_compress/compress/_prepare_strip/_prepare_tile/_compress_block.zlib.compressso the parallel writer paths actually scale across threads; existing tests covered round-trip correctness and pool dispatch but never observed which deflate backend ran or what flag value the writer passed.xrspatial/geotiff/tests/test_gil_friendly_kwarg_1830.py. No source changes.What the new tests pin
deflate_compress(gil_friendly=True)bypasses libdeflate even when the optionaldeflatePyPI binding is installed;gil_friendly=Falsekeeps using it; both round-trip through stdlibzlib.decompressat levels 1/6/9.UserWarningfires exactly once when libdeflate is missing, is suppressed when the module-global latch is set, and is not re-emitted across subsequent calls (no per-call spam).compress(DEFLATE, gil_friendly=...)forwards the flag; the flag is a no-op for LZW/PackBits/zstd/lz4/none._write_strippedparallel path passesgil_friendly=Trueon every strip; the sequential path leaves it atFalse. Same matrix for_write_tiled._prepare_stripand_prepare_tileforward the kwarg intodeflate_compress. The parallel_write_tiledbranch passesTruepositionally to_prepare_tile, pinned via the function signature.Verification
pytest xrspatial/geotiff/tests/test_gil_friendly_kwarg_1830.py-> 20 passed.test_parallel_writer_1800.py+test_compression.py+test_compression_level.py+test_streaming_write.py+ new file -> 99 passed.and not gil_friendlyguard in_compression.py:137-> 2 tests red (test_deflate_compress_gil_friendly_true_bypasses_libdeflate,test_compress_forwards_gil_friendly_to_deflate).gil_friendly=TruetoFalsein_write_stripped->test_write_stripped_parallel_path_uses_gil_friendlyred.warnings.warnblock ->test_deflate_compress_fallback_warning_fires_when_libdeflate_missingred.Test plan
pytest xrspatial/geotiff/tests/test_gil_friendly_kwarg_1830.py -q-> 20 passed on a host withdeflateinstalled.deflate-- the libdeflate-installed tests skip cleanly, the fallback tests stay green.