geotiff: reject zero-band 3D writes at the entry point (#2095)#2096
Merged
Conversation
The 3D spatial-shape validator checked height and width but not the band axis. A DataArray of shape (0, y, x) band-first or (y, x, 0) band-last passed every guard and reached IFD assembly with samples_per_pixel == 0; the resulting file read back as a 2D single-band raster, hiding the upstream collapse of the band axis. Extend _validate_writer_spatial_shape to also reject bands == 0 on both layouts. All public writer entry points (to_geotiff, write, write_streaming, write_geotiff_gpu) already call this helper, so the check fires before any bytes hit disk on every path. Closes #2095
brendancol
commented
May 19, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: geotiff: reject zero-band 3D writes at the entry point (#2095)
Blockers
None.
Suggestions
-
xrspatial/geotiff/tests/test_to_geotiff_zero_bands_2095.py:75,91,123: the assertionsassert "write" in msg/assert "write_streaming" in msg/assert "write_geotiff_gpu" in msgare weak. Every entry-point message contains the substring "write" (the error template says "write a raster with no bands"), so the bare-writedirect-call test cannot tellentry_point="write"apart from"write_streaming"or"write_geotiff_gpu". Anchor to the message prefix (e.g.msg.startswith("write ")for the bare-writetest) or assert the exact entry_point name followed by " cannot write" to guarantee the right path actually fired.
Nits
-
xrspatial/geotiff/_validation.py:166-174: into_geotiffthis spatial validator runs before_validate_3d_writer_dims(#1812 / #1972). For an array of shape(5, 5, 0)with dims('y', 'x', 'time'), this validator now infersbands = 0(since'time'is not in_BAND_DIM_NAMES, it falls to the band-last branch) and raises the "no bands" error rather than the friendlier ambiguous-dim / temporal-dim message from_validate_3d_writer_dims. The rejection is still correct, but the message points at the wrong root cause. Not blocking, since both messages name the right call to fix, but worth a comment or a reorder if you want the ambiguous-dim message to win.
What looks good
- One fail-closed gate, called from every public writer entry point that already calls the spatial-shape validator. No caller-side changes needed.
- Test coverage hits both layouts on numpy + dask, plus direct
write,write_streaming, andwrite_geotiff_gpu. Tests usetmp_pathfixtures with issue-numbered filenames so they cannot collide. - Mirrors the #2075 fail-closed pattern, including the
entry_pointmessage threading. - Validator runs before any IFD math or device dispatch, so no bytes hit disk and no GPU work starts.
Checklist
- Algorithm matches reference/paper — N/A (validator)
- All implemented backends produce consistent results
- NaN handling is correct — N/A
- Edge cases are covered by tests
- Dask chunk boundaries handled correctly
- No premature materialization or unnecessary copies
- Benchmark exists or is not needed — not needed (input rejection)
- README feature matrix updated (if applicable) — N/A
- Docstrings present and accurate
…dator ordering (#2095) - Replace weak ``"write" in msg`` / ``"write_streaming" in msg`` / ``"write_geotiff_gpu" in msg`` checks with ``msg.startswith(...)`` on the exact entry-point prefix so a wrong path firing would now fail the test. The error template starts with ``"<entry_point> cannot write a raster ..."``, so anchoring to the prefix is the cleanest invariant. - Add a docstring note to ``_validate_writer_spatial_shape`` covering the ordering interaction with ``_validate_3d_writer_dims``: a ``(5, 5, 0)`` array with dims ``('y', 'x', 'time')`` now triggers the "no bands" error rather than the ambiguous-dim message. Both rejections are correct; the comment explains the trade-off for future readers.
brendancol
commented
May 19, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up: review findings disposition
- Suggestion (test assertions): Fixed. Replaced
assert "write" in msg/assert "write_streaming" in msg/assert "write_geotiff_gpu" in msgwithmsg.startswith("<entry_point> cannot write"). The error template starts with"<entry_point> cannot write a raster ...", so anchoring to the prefix means a wrong path firing now fails the test. - Nit (validator ordering): Documented. Added a docstring note to
_validate_writer_spatial_shapeexplaining that this validator runs before_validate_3d_writer_dims, so an ambiguous-dim array with a zero-length axis (e.g.(5, 5, 0)with('y', 'x', 'time')) now triggers the "no bands" message rather than the ambiguous-dim diagnostic. Both messages name the right call to fix, so I kept the ordering and explained the trade-off rather than reordering.
Tests re-run: test_to_geotiff_zero_bands_2095.py (7/7 pass), test_to_geotiff_empty_shape_2075.py (5/5), test_to_geotiff_3d_dim_validation_1812.py (19/19).
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
(0, y, x)band-first or(y, x, 0)band-last) reached IFD assembly withsamples_per_pixel == 0and produced a TIFF that round-tripped as a 2D single-band raster._validate_writer_spatial_shapeinxrspatial/geotiff/_validation.pyto also rejectbands == 0on both layouts. The check runs before any bytes are written.to_geotiff,write,write_streaming,write_geotiff_gpu) already calls this helper, so no caller-side changes were needed.Backend coverage
write_streamingdirect plusto_geotiffdask path)Test plan
pytest xrspatial/geotiff/tests/test_to_geotiff_zero_bands_2095.pypytest xrspatial/geotiff/tests/test_to_geotiff_empty_shape_2075.py(regression)pytest xrspatial/geotiff/tests/test_to_geotiff_3d_dim_validation_1812.py(regression)Closes #2095