Skip to content

geotiff: require tile_size to be a multiple of 16 (#1767)#1781

Merged
brendancol merged 3 commits into
mainfrom
issue-1767
May 13, 2026
Merged

geotiff: require tile_size to be a multiple of 16 (#1767)#1781
brendancol merged 3 commits into
mainfrom
issue-1767

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #1767.

Summary

  • to_geotiff and write_geotiff_gpu now refuse tile_size values that aren't a positive multiple of 16 when tiled=True. TIFF 6 spec requires TileWidth and TileLength to be multiples of 16, and other tools (GDAL, libtiff strict mode) may reject files that violate it.
  • The error names the bad value and suggests the nearest valid choices ((tile_size // 16) * 16 and the next multiple up).
  • Docstrings on to_geotiff updated to call out the constraint.

Test plan

  • New tests in xrspatial/geotiff/tests/test_tile_size_multiple_of_16_1767.py cover tile_size=17, tile_size=1, tile_size=8 (sub-16 hint), tile_size=24 (both-sides hint), tile_size=16, tile_size=128, tile_size=256, tile_size=512, and tile_size=17 with tiled=False (must pass).
  • test_size_param_validation_1752.py::test_to_geotiff_tile_size_one_still_writes was inverted (it asserted tile_size=1 worked; that's now disallowed) and replaced with a tile_size=16 positive-path check.
  • Existing fixtures that wrote files with tile_size=2/4/8/100 for multi-tile coverage were bumped to multiples of 16; raster sizes grew only where the test logic depended on a >1-tile grid.
  • Full geotiff test suite passes apart from three pre-existing Python 3.14 matplotlib palette deepcopy failures unrelated to this change.

TIFF 6 spec requires TileWidth and TileLength to be multiples of 16
for broad reader interop. The in-repo writer/reader round-tripped any
positive tile_size (e.g. 17), but other tools (GDAL, libtiff strict
mode) may reject the file.

`to_geotiff` and `write_geotiff_gpu` now refuse tile_size that is not
a positive multiple of 16 when tiled output is requested, naming the
bad value and suggesting the nearest valid choices. Docstrings updated.

Existing test fixtures that used small tile sizes (2/4/8/100) for
multi-tile coverage have been bumped to the nearest valid multiple of
16, with raster sizes adjusted only where the test logic required >1
tile.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 13, 2026
@brendancol brendancol requested a review from Copilot May 13, 2026 12:47
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

This PR enforces TIFF 6 tile dimension requirements in the public GeoTIFF writers by rejecting tile_size values that are not positive multiples of 16 when tiled output is used, improving interoperability with strict readers (e.g., GDAL/libtiff). It also updates and extends the test suite to reflect the new constraint across CPU, GPU, COG, and VRT paths.

Changes:

  • Add tile_size validation (positive multiple of 16) to to_geotiff (when tiled=True) and to write_geotiff_gpu (always tiled), with error hints suggesting nearest valid multiples.
  • Update to_geotiff docstring to document the new constraint.
  • Adjust existing tests/fixtures that used non-compliant tile sizes and add a new regression test module for #1767.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
xrspatial/geotiff/init.py Adds tile size validation for TIFF 6 compliance in CPU and GPU public writers; updates to_geotiff docs.
xrspatial/geotiff/tests/test_tile_size_multiple_of_16_1767.py New regression tests ensuring invalid tile_size values are rejected (and hints are present) for to_geotiff.
xrspatial/geotiff/tests/test_size_param_validation_1752.py Updates positive-path validation test to use the new minimum legal tile size (16).
xrspatial/geotiff/tests/test_vrt_write.py Updates VRT tiling test to use a spec-compliant tile size.
xrspatial/geotiff/tests/test_vrt_tiled_metadata_1606.py Updates VRT tiled-metadata parity tests to use compliant tile sizes.
xrspatial/geotiff/tests/test_streaming_codecs_2026_05_11.py Updates COG/overview streaming codec tests to use compliant tile sizes.
xrspatial/geotiff/tests/test_predictor_multisample.py Adjusts predictor multisample tests to keep multi-tile coverage with compliant tile sizes.
xrspatial/geotiff/tests/test_overview_resampling_min_max_median_2026_05_11.py Updates overview resampling tests (CPU/GPU) to use compliant tile sizes.
xrspatial/geotiff/tests/test_open_geotiff_vrt_kwarg_drop_1685.py Updates COG+overview test setup to use a compliant tile size.
xrspatial/geotiff/tests/test_open_geotiff_on_gpu_failure_1615.py Updates fixture writer call to use a compliant tile size.
xrspatial/geotiff/tests/test_kwarg_coverage_2026_05_11_r4.py Updates small TIFF fixture tile size and adjusts max_pixels budget accordingly.
xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12.py Updates GPU BigTIFF behavior tests to use compliant tile sizes.
xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12_v2.py Updates GPU predictor behavior/parity tests to use compliant tile sizes.
xrspatial/geotiff/tests/test_gpu_writer_overview_mode_and_compression_level_1740.py Updates GPU overview mode/resampling tests to use compliant tile sizes.
xrspatial/geotiff/tests/test_gpu_window_band_1605.py Updates GPU window/band fixtures to use compliant tile sizes.
xrspatial/geotiff/tests/test_features.py Updates BigTIFF tile-offset coverage tests to use compliant tile sizes.
xrspatial/geotiff/tests/test_cog_overview_nodata_1613.py Updates COG overview nodata tests (CPU/GPU) to use compliant tile sizes.
xrspatial/geotiff/tests/test_cog_cubic_overview_nodata_1623.py Updates COG cubic overview nodata tests (CPU/GPU) to use compliant tile sizes.
xrspatial/geotiff/tests/test_band_validation_1673.py Updates multiband fixture writer call to use a compliant tile size.
xrspatial/geotiff/tests/test_backend_kwarg_parity_1561.py Updates backend parity test to use a compliant tile size.
Comments suppressed due to low confidence (2)

xrspatial/geotiff/init.py:3291

  • The new tile_size multiple-of-16 validation in write_geotiff_gpu changes the public contract, but the write_geotiff_gpu docstring still says only “Tile size in pixels”. Please update the docstring to state that tile_size must be a positive multiple of 16 (TIFF 6 requirement), matching to_geotiff’s docs so callers don’t discover this via runtime errors.
    # Mirror to_geotiff's tile_size validation: the TIFF 6 spec requires
    # TileWidth/TileLength to be positive multiples of 16 for broad
    # reader interop. ``write_geotiff_gpu`` is always tiled, so the
    # check fires unconditionally here.

xrspatial/geotiff/init.py:3300

  • This new error path in write_geotiff_gpu (rejecting non-multiples of 16) doesn’t appear to have a dedicated regression test. Since the validation happens before the cupy import, you can add a CPU-only test asserting write_geotiff_gpu(..., tile_size=17) raises ValueError, which will run even when CUDA/cupy isn’t available.
    if not isinstance(tile_size, (int, np.integer)) or isinstance(
            tile_size, bool):
        raise ValueError(
            f"tile_size must be a positive int, got "
            f"{tile_size!r} (type {type(tile_size).__name__}).")
    if tile_size <= 0:
        raise ValueError(
            f"tile_size must be a positive int, got tile_size={tile_size}.")
    if tile_size % 16 != 0:

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

Comment thread xrspatial/geotiff/__init__.py Outdated
Comment on lines 1277 to 1285
@@ -1277,6 +1283,19 @@ def to_geotiff(data: xr.DataArray | np.ndarray,
if tile_size <= 0:
raise ValueError(
f"tile_size must be a positive int, got tile_size={tile_size}.")
Both to_geotiff and write_geotiff_gpu were running the same three
tile_size validations (type/bool guard, positivity, multiple-of-16
hint). Routing both call sites through a shared _validate_tile_size
helper keeps accepted types and error wording in lockstep so future
edits cannot drift between the two entry points.

Also documents the multiple-of-16 requirement in the write_geotiff_gpu
docstring and adds CPU-only regression tests for the GPU writer's
tile_size validation, which runs before any cupy import.
@brendancol
Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: brendancol <433221+brendancol@users.noreply.github.com>
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.

to_geotiff accepts tile_size values that are not multiples of 16

3 participants