Skip to content

Cover compression_level= for compression='lz4'#1650

Merged
brendancol merged 1 commit into
xarray-contrib:mainfrom
brendancol:deep-sweep-test-coverage-geotiff-2026-05-11-01
May 12, 2026
Merged

Cover compression_level= for compression='lz4'#1650
brendancol merged 1 commit into
xarray-contrib:mainfrom
brendancol:deep-sweep-test-coverage-geotiff-2026-05-11-01

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

The compression-level validator in xrspatial.geotiff.__init__ advertises a (0, 16) valid range for lz4 alongside deflate (1, 9) and zstd (1, 22), but only the deflate and zstd ranges had round-trip + boundary-reject tests.

The lz4 path goes through the same _LEVEL_RANGES validator at three call sites (eager numpy, dask streaming, and _write_vrt_tiled). A regression that dropped 'lz4' from _LEVEL_RANGES would have silently accepted any int level: lz4_compress itself does not validate the level argument.

This adds 18 tests in xrspatial/geotiff/tests/test_lz4_compression_level_2026_05_11.py:

  • Round-trip at levels 0 / 1 / 9 / 16 (lossless: assert_array_equal).
  • Default no-arg path (compression_level=None falls through to the codec default).
  • Higher level not larger on compressible input (level 16 file size <= level 0 file size).
  • Eager out-of-range reject at -1 / -10 / 17 / 100.
  • Valid-range message format pin (lz4 ... valid: 0-16).
  • Dask streaming round-trip at 0 / 1 / 8 / 16 (separate validation call site).
  • Dask streaming out-of-range reject at -1 / 17 / 50.

Closes the Cat 4 MEDIUM parameter-coverage gap left after pass 7 of the test-coverage sweep on geotiff. No source changes.

Test plan

  • All 18 new tests pass locally
  • No regressions in test_compression_level.py (14 tests) or test_lz4.py (13 tests)
  • Verified the test suite would catch a regression: dropping 'lz4' from _LEVEL_RANGES makes the out-of-range tests fail (silent acceptance)

The compression-level validator in xrspatial.geotiff.__init__ advertises
a (0, 16) valid range for lz4 alongside deflate (1, 9) and zstd (1, 22),
but only the deflate and zstd ranges had round-trip + boundary-reject
tests. The lz4 path goes through the same validator at three call sites
(eager numpy, dask streaming, and _write_vrt_tiled), so a regression
that dropped 'lz4' from _LEVEL_RANGES would have silently accepted
any int level -- lz4_compress itself does not validate. Adds 18 tests:
round-trip at 0/1/9/16 (lossless), default no-arg path, higher-level
not larger on compressible input, eager out-of-range reject at
-1/-10/17/100, valid-range message format pin, dask streaming
round-trip at 0/1/8/16, and dask streaming out-of-range reject at
-1/17/50.

Closes the Cat 4 MEDIUM parameter-coverage gap left after pass 7.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 12, 2026
@brendancol brendancol requested a review from Copilot May 12, 2026 01:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds targeted test coverage for compression='lz4' when using the compression_level= parameter in the GeoTIFF writer, ensuring the documented (0, 16) level range is enforced and that both eager and dask-streaming call sites are covered.

Changes:

  • Add a new test module covering LZ4 round-trip correctness at boundary and representative levels, default compression_level=None behavior, out-of-range rejection, and dask-streaming validation.
  • Update the internal sweep state CSV to record the added coverage work.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
xrspatial/geotiff/tests/test_lz4_compression_level_2026_05_11.py New tests for LZ4 compression_level range enforcement, error messaging, and eager + dask streaming round-trips.
.claude/sweep-test-coverage-state.csv Records the new “Pass 8” test-coverage sweep status entry for geotiff.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@brendancol brendancol merged commit 6f4b2d1 into xarray-contrib:main May 12, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants