Skip to content

geotiff: drop decorative post-write header check in writer#1848

Merged
brendancol merged 1 commit into
mainfrom
issue-1844
May 14, 2026
Merged

geotiff: drop decorative post-write header check in writer#1848
brendancol merged 1 commit into
mainfrom
issue-1844

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #1844.

Summary

  • Removes the "post-write validation" block from write_geotiff and the streaming writer in xrspatial/geotiff/_writer.py.
  • Both copies reparsed the first 16 bytes after the file (or temp file) was already on disk and emitted a Written file may be corrupt warning on failure.

Why

  • The check only covered the TIFF / BigTIFF header that our own builder produced, so it said nothing about IFDs, tile or strip offsets and byte counts, geotags, COG layout, or whether any tile decodes.
  • It ran after _write_bytes (or after the temp file was fully written), so even a real failure would have left a bad file on disk; the warning misled callers into thinking the file had been verified.
  • The TIFF assembly is internal and type-checked. A genuine round-trip verifier would duplicate the reader and is better left to callers who can simply call read_geotiff(path).

What callers gain or lose

  • Gain: clearer semantics. The writer no longer claims a verification it does not actually perform.
  • Lose: nothing meaningful. No test asserted on this warning, and the parse of 16 self-produced bytes was effectively always succeeding.

Test plan

  • pytest xrspatial/geotiff/tests/test_writer.py xrspatial/geotiff/tests/test_writer_matrix.py xrspatial/geotiff/tests/test_streaming_write.py xrspatial/geotiff/tests/test_parallel_writer_1800.py -- 145 passed.
  • Full pytest xrspatial/geotiff/tests/ matches main: the remaining failures (test_features.py::TestPalette, test_predictor2_big_endian_gpu_1517.py, test_size_param_validation_gpu_vrt_1776.py) reproduce on main and are unrelated to this change.

Both `write_geotiff` and the streaming writer reparsed the first 16
bytes after the file (or temp file) was already written and emitted a
"Written file may be corrupt" warning on failure. The check only
covered the TIFF/BigTIFF header that our own builder produced, so it
proved almost nothing about IFDs, tile or strip offsets, geotags, or
COG layout, and the warning fired after the bytes hit disk anyway.

Drop both blocks. Callers who want a real round-trip integrity check
can call `read_geotiff(path)` themselves.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 14, 2026
@brendancol brendancol requested a review from Copilot May 14, 2026 16: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 removes the “post-write validation” blocks in the GeoTIFF writers that reparsed the first 16 bytes (TIFF/BigTIFF header) after the file had already been written, and emitted a potentially misleading Written file may be corrupt warning.

Changes:

  • Removed the post-write header parse/warn block from the eager write() path.
  • Removed the equivalent post-write header parse/warn block from write_streaming() (temp-file path before os.replace).

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

@brendancol brendancol merged commit 83f90dc into main May 14, 2026
16 checks passed
@brendancol brendancol deleted the issue-1844 branch May 15, 2026 04:38
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.

Drop decorative post-write TIFF header check in writer

2 participants