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(),