Forward window/band and handle planar=2 on HTTP COG reads (#1669)#1680
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes parity gaps between local and HTTP GeoTIFF/COG reads in xrspatial.geotiff, ensuring open_geotiff(url, window=..., band=...) behaves consistently across backends and correctly handles PlanarConfiguration=2 (separate planes) in HTTP tile fetch/decoding.
Changes:
- Forward
windowthrough the HTTP COG eager path (read_to_array→_read_cog_http→_fetch_decode_cog_http_tiles) and validate it against IFD extent. - Apply
bandselection consistently on the HTTP eager path after materializing the array. - Add PlanarConfiguration=2 support to the HTTP tile-indexing/decoding loop and add parity tests for window/band/planar=2.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xrspatial/geotiff/_reader.py |
Threads window into HTTP reads, validates window, slices band, and fixes planar=2 tile indexing/placement for HTTP tile decoding. |
xrspatial/geotiff/tests/test_http_window_band_planar_1669.py |
Adds HTTP-vs-local parity tests covering window, band, window+band, and planar=2 behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1598
to
+1607
| # Validate ``window`` against the selected IFD's extent before the | ||
| # tile fetch is built. Without this, the helper silently clamps an | ||
| # out-of-bounds window and returns a smaller array, mismatching | ||
| # ``open_geotiff``'s caller-built coord arrays. Mirrors the | ||
| # local-path validator in ``read_to_array`` (#1634). | ||
| if window is not None: | ||
| w_r0, w_c0, w_r1, w_c1 = window | ||
| if (w_r0 < 0 or w_c0 < 0 | ||
| or w_r1 > ifd.height or w_c1 > ifd.width | ||
| or w_r0 >= w_r1 or w_c0 >= w_c1): |
Comment on lines
+213
to
+226
| tifffile = pytest.importorskip('tifffile') | ||
| h, w, bands = 32, 48, 3 | ||
| rng = np.random.RandomState(1669) | ||
| data = rng.randint(0, 200, size=(bands, h, w)).astype(np.uint8) | ||
| expected = np.transpose(data, (1, 2, 0)) | ||
| path = str(tmp_path / 'tmp_1669_chunky.tif') | ||
| tifffile.imwrite( | ||
| path, | ||
| expected, | ||
| photometric='rgb', | ||
| planarconfig='contig', | ||
| tile=(16, 16), | ||
| compression='deflate', | ||
| ) |
Comment on lines
+301
to
+314
| tifffile = pytest.importorskip('tifffile') | ||
| h, w, bands = 32, 48, 3 | ||
| rng = np.random.RandomState(0x16692) | ||
| data = rng.randint(0, 200, size=(bands, h, w)).astype(np.uint8) | ||
| # tifffile with planarconfig='separate' expects (bands, H, W) input. | ||
| path = str(tmp_path / 'tmp_1669_planar2.tif') | ||
| tifffile.imwrite( | ||
| path, | ||
| data, | ||
| photometric='rgb', | ||
| planarconfig='separate', | ||
| tile=(16, 16), | ||
| compression='deflate', | ||
| ) |
This was referenced May 12, 2026
brendancol
added a commit
that referenced
this pull request
May 12, 2026
Two PR #1680 review items: 1. _read_cog_http now mirrors the local-path Orientation tag (274) rejection in read_to_array: a windowed read against a non-default Orientation has ambiguous semantics and HTTP does not yet implement _apply_orientation. Without the guard, HTTP could silently return a different region or pixel order than the local path for the same file. Add a parity test reading an Orientation=2 TIFF over both transports. 2. The planar=1 multi-band fixture now uses the project's own writer instead of tifffile, and the planar=2 fixture builds the TIFF bytes directly (the project writer only emits planar=1). The oriented-TIFF fixture is also hand-rolled. The four pytest.importorskip("tifffile") gates are removed so all 12 tests run in the default test environment (CI was previously skipping the multi-band and planar=2 parity coverage).
#1669) ``open_geotiff(url, window=..., band=...)`` accepted both kwargs but the HTTP branch dropped them. The local path honoured them, so the same call on a file:// path versus an http:// path returned different shapes. The HTTP tile-index loop also ignored ``PlanarConfiguration=2``, fetching band 0's byte ranges for every band on separate-plane COGs. * ``read_to_array`` now passes ``window`` to ``_read_cog_http``, which in turn passes it to ``_fetch_decode_cog_http_tiles`` (the helper already accepted it; it was just being hardcoded to None). * ``_read_cog_http`` applies the same ``arr[:, :, band]`` slice as the local path after the array is materialised. * ``_read_cog_http`` validates the window against the IFD extent before the fetch, matching the local-path validator from #1634. * ``_fetch_decode_cog_http_tiles`` mirrors the planar=2 logic from ``_read_tiles``: ``band_count = samples`` when ``planar == 2``, ``tile_idx = band_idx * tiles_per_band + tr * tiles_across + tc``, and each per-band tile decodes with ``samples=1`` and lands in the ``[..., band_idx]`` slot of the output buffer. Adds parity tests against a loopback http.server: windowed reads, band-selected reads, window+band combined, full-image planar=2 reads, windowed planar=2 reads, and band-selected planar=2 reads. The dask HTTP path uses the same helper and picks up the planar=2 fix automatically.
Two PR #1680 review items: 1. _read_cog_http now mirrors the local-path Orientation tag (274) rejection in read_to_array: a windowed read against a non-default Orientation has ambiguous semantics and HTTP does not yet implement _apply_orientation. Without the guard, HTTP could silently return a different region or pixel order than the local path for the same file. Add a parity test reading an Orientation=2 TIFF over both transports. 2. The planar=1 multi-band fixture now uses the project's own writer instead of tifffile, and the planar=2 fixture builds the TIFF bytes directly (the project writer only emits planar=1). The oriented-TIFF fixture is also hand-rolled. The four pytest.importorskip("tifffile") gates are removed so all 12 tests run in the default test environment (CI was previously skipping the multi-band and planar=2 parity coverage).
This was referenced May 12, 2026
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.
Fixes #1669.
Summary
open_geotiff(url, window=..., band=...)accepted both kwargs, but the HTTP branch silently dropped them. The local path used them, so the same call onfile://versushttp://returned arrays of different shapes (the HTTP path returned the full raster whileopen_geotiffbuilt coords sized to the window, raisingCoordinateValidationErroron shape mismatch). The HTTP tile-index loop also ignoredPlanarConfiguration=2, so separate-plane COGs fetched the wrong byte ranges and decoded garbage.What changed
xrspatial/geotiff/_reader.py:read_to_arraynow passeswindowto_read_cog_http._read_cog_httpacceptswindow, validates it against the IFD extent (same message format as the local path's open_geotiff eager path windowed read coord/data shape mismatch on out-of-bounds windows #1634 validator), and forwards it to_fetch_decode_cog_http_tiles(which already supported it; it was hardcoded to None at the only call site)._read_cog_httpappliesarr[:, :, band]after materialisation, matching the local path slice inread_to_array._fetch_decode_cog_http_tileshandlesPlanarConfiguration=2by iterating bands and computingtile_idx = band_idx * tiles_per_band + tr * tiles_across + tc, decoding each per-band tile withsamples=1, and placing it into the[..., band_idx]slot of the output. Mirrors the existing_read_tilesplanar=2 logic.The dask HTTP path uses the same
_fetch_decode_cog_http_tileshelper and picks up the planar=2 fix automatically. I verified end-to-end withread_geotiff_daskagainst a planar=2 tiled file.Tests
xrspatial/geotiff/tests/test_http_window_band_planar_1669.py(11 new tests). Each builds a small tiled file, serves it viahttp.serverwith HTTP Range support, and compares the HTTP read against the local read pixel-for-pixel:read_to_arrayand_read_cog_httpwindow parityopen_geotiffandread_to_arrayAll 11 new tests fail against the pre-fix code and pass after.
Test plan
pytest xrspatial/geotiff/tests/test_http_window_band_planar_1669.pypytest xrspatial/geotiff/tests/test_cog_http_concurrent.py test_http_cog_coalesce.py test_ssrf_hardening_1664.py test_cog.py test_planar_multiband.py test_window_out_of_bounds_1634.py test_dask_planar_multiband.pyTestPalettefailures (pre-existing matplotlib recursion, present on main)