feat(rust/sedona-raster): BandRef::copy_into derive-with-overrides (zero-copy)#964
Merged
Merged
Conversation
…ero-copy)
Add a band-driven derive API: BandRef::copy_into(builder, BandOverrides{..})
writes a derived band, inheriting unspecified fields (dim names, shape, data
type, nodata, OutDb pointers) from the source and carrying its bytes over. The
data step is a separate append_data_into trait method — default copies via
append_value; BandRefImpl overrides it to share the source row's backing Buffer
zero-copy (append_band_data_from), keeping the Arrow buffer plumbing
encapsulated (no raw Buffer accessor on the trait).
Identity-view only: the derived band uses start_band_nd. View-carrying
overrides land with the view machinery. First step of DB-81; consumers
(RS_EnsureLoaded rebuild, etc.) migrate next.
Member
|
@james-willis Is this ready to review? |
Contributor
Author
No, its not prioritized for 0.4.0 so I haven't finished it. Its part of the lazy slicing feature which will land later. |
copy_into emits an identity-view band and copies the source's visible bytes assuming offset 0 / canonical strides. Guard that precondition at the single dispatch point (covering both the default and the Arrow append_band_data_from data paths): a non-identity source view now errors loudly instead of silently copying mislocated bytes. Carrying views is a follow-up (the view machinery).
Replace the in-copy_into non-identity guard with a future-proof seam: - Add RasterBuilder::start_band_nd_with_view, which persists an explicit view (identity -> the canonical null sentinel; non-identity -> errors, gated until view persistence lands, apache#897). - copy_into now composes BandOverrides.view onto the source's own view and forwards the result, so callers express overrides in the source's visible coordinates and never manage composition themselves. - Add BandOverrides.view. When non-identity view persistence lands, only the builder + reader + loader change — copy_into, append_data_into, and BandOverrides are frozen.
…::try_new main renamed RasterStructArray::new -> try_new; update the copy_into tests (merged in from this branch) to match.
paleolimbot
approved these changes
Jun 23, 2026
paleolimbot
left a comment
paleolimbot
left a comment
Member
There was a problem hiding this comment.
Thank you!
Possibly worth dealing with the clippy allow before you go further (there are a lot of follow-ups already)
Comment on lines
+404
to
+415
| #[allow(clippy::too_many_arguments)] | ||
| pub fn start_band_nd_with_view( | ||
| &mut self, | ||
| name: Option<&str>, | ||
| dim_names: &[&str], | ||
| shape: &[i64], | ||
| data_type: BandDataType, | ||
| nodata: Option<&[u8]>, | ||
| outdb_uri: Option<&str>, | ||
| outdb_format: Option<&str>, | ||
| view: &[ViewEntry], | ||
| ) -> Result<(), ArrowError> { |
Member
There was a problem hiding this comment.
Clippy is right about this one, and it's a public API and probably worth dealing with before it is used. A common way to handle this is to have a struct NewBandArgs<'a> { ... }, which forces code to name arguments.
…struct Replace the eight positional arguments (which required #[allow(clippy::too_many_arguments)]) with a named StartBandNdWithViewArgs struct so call sites spell out each field.
…art_band_nd_with_view The builder method only checked the view was identity and delegated to start_band_nd; since the band schema can't persist a non-identity view yet, it added public API for no storage behavior. Fold the identity check into copy_into and call start_band_nd directly, removing the method and its StartBandNdWithViewArgs struct.
…th_view seam Replace the inline identity gate with a pub(crate) start_band_with_view that takes a StartBandWithViewArgs struct, matching the entry point view persistence will use. copy_into routes through it unchanged once persistence lands; today the method rejects a non-identity view since the schema stores views only as the identity null sentinel.
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
3 participants
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.
No description provided.