You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sedona.apache.org by ja...@apache.org on 2026/08/26 01:03:24 UTC
(sedona-db) 01/01: fix(rust/sedona-raster): carry band names through copy_raster_from
This is an automated email from the ASF dual-hosted git repository.
james-willis pushed a commit to branch james/db-506-rs_setgeoreference-drops-band-names
in repository https://gitbox.apache.org/repos/asf/sedona-db.git
commit c299d1199b50166fa210ea6eaed2c9fe0cd62451
Author: jameswillis <ja...@wherobots.com>
AuthorDate: Tue Aug 25 18:03:15 2026 -0700
fix(rust/sedona-raster): carry band names through copy_raster_from
`RasterBuilder::copy_raster_from` passed `BandOverrides::default()`, whose
`name` is `None`. `copy_into` cannot inherit the name — it lives on
`RasterRef::band_name(index)`, not on `BandRef` — so every copied band came
out unnamed.
`RS_SetGeoReference` is the only production caller, so it silently dropped
all band names, which is user-visible for named bands such as Zarr variable
names.
Pass `source.band_name(band_idx)` explicitly in the copy loop.
The existing `copy_raster_from_overrides_transform_and_preserves_bands` test
used an unnamed band, and `assert_raster_arrays_equal` never compared band
names at all, so nothing caught this. Add the missing band-name assertion to
the shared comparator — it passes across every raster crate, confirming this
was the only drop — and give the affected fixtures real names so the
regression is covered end to end.
---
.../src/rs_set_georeference.rs | 28 ++++++++++++++++++++--
rust/sedona-raster/src/builder.rs | 19 +++++++++++----
rust/sedona-testing/src/rasters.rs | 5 ++++
3 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/rust/sedona-raster-functions/src/rs_set_georeference.rs b/rust/sedona-raster-functions/src/rs_set_georeference.rs
index 6b0ba72c4..d603a5c41 100644
--- a/rust/sedona-raster-functions/src/rs_set_georeference.rs
+++ b/rust/sedona-raster-functions/src/rs_set_georeference.rs
@@ -206,11 +206,12 @@ mod tests {
use sedona_testing::raster_spec::{assert_rasters_equal, raster_array, RasterSpec};
use sedona_testing::testers::ScalarUdfTester;
- /// A 2x2 raster with a CRS — so the comparisons confirm the CRS (and pixels)
- /// survive the transform swap.
+ /// A 2x2 raster with a CRS and a named band — so the comparisons confirm
+ /// the CRS, the band name, and the pixels all survive the transform swap.
fn base() -> RasterSpec {
RasterSpec::d2(2, 2)
.band_values(&[1u8, 2, 3, 4])
+ .name("temperature")
.crs(Some("OGC:CRS84"))
}
@@ -263,6 +264,29 @@ mod tests {
assert_rasters_equal(&result, &[Some(expected)]);
}
+ #[test]
+ fn preserves_per_band_names_across_multiple_bands() {
+ // Band names live on the raster (`RasterRef::band_name(i)`), not on
+ // `BandRef`, so the rebuild has to carry each one across by index.
+ // Two differently-named bands pin the indexing, not just the presence
+ // of a name.
+ let multi = || {
+ RasterSpec::d2(2, 2)
+ .band_values(&[1u8, 2, 3, 4])
+ .name("temperature")
+ .band_values(&[5u8, 6, 7, 8])
+ .name("precipitation")
+ .crs(Some("OGC:CRS84"))
+ };
+
+ let result = tester_2arg()
+ .invoke_array_scalar(Arc::new(multi().build()), "2 0 0 -3 100 200")
+ .unwrap();
+
+ let expected = multi().transform([100.0, 2.0, 0.0, 200.0, 0.0, -3.0]);
+ assert_rasters_equal(&result, &[Some(expected)]);
+ }
+
#[test]
fn null_georef_nulls_raster() {
let result = tester_2arg()
diff --git a/rust/sedona-raster/src/builder.rs b/rust/sedona-raster/src/builder.rs
index a02d09041..8974abf4d 100644
--- a/rust/sedona-raster/src/builder.rs
+++ b/rust/sedona-raster/src/builder.rs
@@ -287,9 +287,17 @@ impl RasterBuilder {
) -> Result<(), ArrowError> {
self.start_raster_from(source, overrides)?;
for band_idx in 0..source.num_bands() {
- source
- .band(band_idx)?
- .copy_into(self, BandOverrides::default())?;
+ // The band name lives on the *raster* (`RasterRef::band_name`), not
+ // on `BandRef`, so `copy_into` has nothing to inherit it from and
+ // must be handed it explicitly — otherwise every copied band comes
+ // out unnamed.
+ source.band(band_idx)?.copy_into(
+ self,
+ BandOverrides {
+ name: source.band_name(band_idx),
+ ..BandOverrides::default()
+ },
+ )?;
self.finish_band()?;
}
self.finish_raster()
@@ -883,6 +891,7 @@ mod tests {
// Source: a CRS, a nodata sentinel, and pixel values to preserve.
let source = RasterSpec::d2(2, 1)
.band_values(&[1u8, 2])
+ .name("temperature")
.nodata(9u8)
.crs(Some("OGC:CRS84"))
.build();
@@ -899,9 +908,11 @@ mod tests {
.unwrap();
let out: ArrayRef = Arc::new(builder.finish().unwrap());
- // Only the transform changed; CRS, nodata, and pixels carried over.
+ // Only the transform changed; CRS, nodata, band name, and pixels
+ // carried over.
let expected = RasterSpec::d2(2, 1)
.band_values(&[1u8, 2])
+ .name("temperature")
.nodata(9u8)
.crs(Some("OGC:CRS84"))
.transform([100.0, 2.0, 0.0, 200.0, 0.0, -3.0]);
diff --git a/rust/sedona-testing/src/rasters.rs b/rust/sedona-testing/src/rasters.rs
index 3331313a4..a06a43dba 100644
--- a/rust/sedona-testing/src/rasters.rs
+++ b/rust/sedona-testing/src/rasters.rs
@@ -440,6 +440,11 @@ pub fn assert_raster_equal(raster1: &impl RasterRef, raster2: &impl RasterRef) {
let band1 = raster1.band(band_index).unwrap();
let band2 = raster2.band(band_index).unwrap();
+ assert_eq!(
+ raster1.band_name(band_index),
+ raster2.band_name(band_index),
+ "Band names do not match"
+ );
assert_eq!(
band1.dim_names(),
band2.dim_names(),