feat(rust/sedona-raster): read a bare band array via BandStructArray - #1179
Closed
james-willis wants to merge 1 commit into
Closed
feat(rust/sedona-raster): read a bare band array via BandStructArray#1179james-willis wants to merge 1 commit into
james-willis wants to merge 1 commit into
Conversation
apache#1161 made band rows *writable* without a raster envelope (`BandArrayBuilder`), but left no way to read them back: `BandRefImpl` is private and the only public readers (`RasterStructArray` / `RasterRefImpl`) require an enclosing raster. This adds the symmetric read half. `BandStructArray::try_new(&StructArray)` borrows a bare band array -- one band per row, no `crs`/`transform`/`spatial_dims`/`spatial_shape` and no per-raster `bands` list -- and `get(i)` returns a real `Box<dyn BandRef>`. The band read path is *shared*, not duplicated. Reading a band depends only on (band-level columns, row index) -- nothing about it is raster-specific -- so the column references plus the construction logic move to a new internal `BandColumns`: view decode and validation, 0-D and unknown-dtype rejection, InDb/OutDb resolution, byte-stride composition via `compose_byte_strides`, and the view-buffer bounds check. `RasterRefImpl::band` now resolves its band's absolute row through the raster's flattened `bands` list offsets and delegates; `BandStructArray::get` delegates with the row index directly. Neither reimplements any of it, so a band cannot drift between the two paths. This is what lets an out-of-tree type that reuses the Band physical layout (e.g. a tensor/chunk value carrying its own georeferencing rather than a raster's) read its data back through the same already-reviewed stride machinery instead of copying it downstream, where it would silently diverge -- including the overflow hardening `compose_byte_strides` carries. No public behavior change: `RasterRefImpl::band` is byte-for-byte equivalent (proven directly by `bare_band_and_in_raster_band_agree`, which reads the *same* bands array both ways and asserts every accessor plus the full `nd_buffer` shape/strides/offset match). Verification: - cargo test -p sedona-raster: 182 passed (178 existing, unmodified, +4 new: bare round-trip through BandArrayBuilder, non-identity view decode with correct strides/offset, the raster-vs-bare equivalence test, and rejection of an out-of-range index and a non-band struct). - sedona-raster-functions (254), sedona-raster-zarr (66), sedona-spatial-join-raster (13), sedona-testing (93): all pass, no regressions. - clippy --all-targets -- -D warnings and fmt --all -- --check: clean.
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
1 participant
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.
What
Adds
BandStructArray— a public reader for a bare bandStructArray(one band per row, no enclosing raster).try_new(&StructArray)borrows it;get(i)returns a realBox<dyn BandRef>.Why
#1161 made band rows writable without a raster envelope (
BandArrayBuilder), but left no way to read them back:BandRefImplis private, and the only public readers (RasterStructArray/RasterRefImpl) require an enclosing raster. This is the symmetric read half.Without it, an out-of-tree type reusing the Band physical layout (e.g. a tensor/chunk value that carries its own georeferencing rather than a raster's) has to reimplement the view→byte-stride derivation downstream — including
compose_byte_strides' overflow hardening — where it would silently drift from this crate.How
Reading a band depends only on
(band-level columns, row index)— nothing about it is raster-specific. So the column references and the construction logic move into an internalBandColumns: view decode + validation, 0-D and unknown-dtype rejection, InDb/OutDb resolution, byte-stride composition, and the view-buffer bounds check.RasterRefImpl::bandresolves its band's absolute row through the raster's flattenedbandslist offsets, then delegates.BandStructArray::getdelegates with the row index directly.Neither reimplements any of it, so the two paths can't diverge.
Behavior change
None.
RasterRefImpl::bandis equivalent — proven directly bybare_band_and_in_raster_band_agree, which reads the same bands array both ways and asserts every accessor plus the fullnd_buffershape/strides/offset agree.Verification
cargo test -p sedona-raster: 182 passed (178 existing and unmodified, +4 new — bare round-trip throughBandArrayBuilder, non-identity view decode with correct strides/offset, the raster-vs-bare equivalence test, and rejection of an out-of-range index and a non-band struct).sedona-raster-functions(254),sedona-raster-zarr(66),sedona-spatial-join-raster(13),sedona-testing(93): all pass, zero regressions.clippy --all-targets -- -D warningsandfmt --all -- --check: clean.sedona-raster-gdalnot buildable on this machine (pre-existing local GDAL/gdal-sysversion mismatch, fails identically on cleanmain); it uses only unchanged public API and doesn't implementBandRef.