refactor(raster): use i64 throughout the view-machinery surface

Per Dewey's review: every `source_axis`/`source_shape`/`visible_shape`
in the view helpers should be `i64` to match `ViewEntry`'s signed
fields and the stride arithmetic, instead of round-tripping through
`u64` and sprinkling `as` casts through helper bodies. Reworked to
make i64 the canonical view-machinery type — no more u64↔i64
conversions in `data()`, `nd_buffer()`, `contiguous_data()`, or the
band-construction storage step.

Public surface changes:

- `BandRef::shape() -> &[i64]` (was `&[u64]`). Production callers
  outside the raster crate's own tests don't exist; tests use array
  literals which infer the new element type. `width()` and `height()`
  stay `u64` (conventional non-negative scalar counts) and pick up a
  single `as u64` cast in the default `dim_size()` impl.
- `BandRef::raw_source_shape() -> &[u64]` unchanged — this is a
  direct slice into the Arrow `UInt64Array` column, changing the
  return type would force an alloc per call.
- `NdBuffer.shape: Vec<i64>`, `NdBuffer.offset: i64` — internal
  struct, no out-of-crate consumers.

Internals now `i64`:

- `validate_view(view, source_shape: &[i64])`, with an added
  `source_shape[sa] >= 0` check at the boundary.
- `is_identity_view(view, source_shape: &[i64])`.
- `visible_shape_from_view(view) -> Vec<i64>`.
- `BandRefImpl::visible_shape: Vec<i64>` (matches the trait return).
- `materialize_strided` and `check_view_buffer_bounds` take
  `visible_shape: &[i64]`.

Conversions concentrated at the two unavoidable boundaries — both
are Arrow column reads:

- `RasterRefImpl::band()` converts `raw_source_shape() -> &[u64]`
  into `Vec<i64>` once with a `try_from` overflow check.
- `RasterBuilder::start_band_with_view` does the same conversion at
  its `source_shape: &[u64]` entry point before calling
  `validate_view`.

Eliminates the source-stride `i64::try_from` cast that used to live
inside `band()` (line ~527 pre-refactor). Tests pass: sedona-raster
131, sedona-raster-functions 143, sedona-raster-gdal 53. Arrow column
storage and the public `width()`/`height()` API unchanged.

Author: james-willis

rust/sedona-raster/src/array.rs

@@ -50,8 +50,11 @@ struct BandRefImpl<'a> {
     data_type: BandDataType,
     /// Per-visible-axis view, length = ndim
     view_entries: Vec<ViewEntry>,
-    /// Visible shape (== `[v.steps for v in view_entries]`), length = ndim
-    visible_shape: Vec<u64>,
+    /// Visible shape (== `[v.steps for v in view_entries]`), length = ndim.
+    /// `i64` to match `BandRef::shape()`'s return type and the surrounding
+    /// view-machinery arithmetic (strides, offsets). `validate_view`
+    /// guarantees entries are non-negative.
+    visible_shape: Vec<i64>,
     /// Byte strides per visible axis. May be 0 (broadcast) or negative.
     byte_strides: Vec<i64>,
     /// Byte offset into `data` of the visible region's `[0,...,0]` element.
@@ -83,7 +86,7 @@ impl<'a> BandRef for BandRefImpl<'a> {
             .collect()
     }
 
-    fn shape(&self) -> &[u64] {
+    fn shape(&self) -> &[i64] {
         &self.visible_shape
     }
 
@@ -168,13 +171,11 @@ impl<'a> BandRef for BandRefImpl<'a> {
         }
         // shape and strides are owned by NdBuffer (see its doc comment).
         // Cloning here is cheap — both vecs are O(ndim), a handful of values.
-        // Cast offset i64 -> u64: safe because RasterRefImpl::band asserts
-        // byte_offset >= 0 before storing.
         Ok(NdBuffer {
             buffer: self.data_array.value(self.band_row),
             shape: self.visible_shape.clone(),
             strides: self.byte_strides.clone(),
-            offset: self.byte_offset as u64,
+            offset: self.byte_offset,
             data_type: self.data_type,
         })
     }
@@ -186,10 +187,7 @@ impl<'a> BandRef for BandRefImpl<'a> {
             // ARE the visible bytes. Borrow them.
             return Ok(Cow::Borrowed(buf.buffer));
         }
-        // Use self.* layout fields rather than `buf.*` to avoid the
-        // i64 -> u64 -> i64 round-trip through NdBuffer for `byte_offset`.
-        // The visible shape and byte strides are precomputed once at
-        // construction; `buf.shape` / `buf.strides` are clones of those.
+        // `buf.shape` is a clone of self.visible_shape; use self.* directly.
         let out = materialize_strided(
             buf.buffer,
             &self.visible_shape,
@@ -220,7 +218,7 @@ impl<'a> BandRef for BandRefImpl<'a> {
 /// and skip the check.
 fn check_view_buffer_bounds(
     buffer_len: usize,
-    visible_shape: &[u64],
+    visible_shape: &[i64],
     byte_strides: &[i64],
     byte_offset: i64,
     dtype_size: usize,
@@ -231,9 +229,9 @@ fn check_view_buffer_bounds(
     let mut min_offset = byte_offset;
     let mut max_offset = byte_offset;
     for (k, &stride) in byte_strides.iter().enumerate() {
-        let last_idx = i64::try_from(visible_shape[k] - 1).map_err(|_| {
-            ArrowError::InvalidArgumentError(format!("visible_shape[{k}] - 1 exceeds i64::MAX"))
-        })?;
+        // `validate_view` guarantees `steps >= 0`, so `visible_shape[k] >= 0`
+        // and `visible_shape[k] - 1` is in-range for any non-empty axis.
+        let last_idx = visible_shape[k] - 1;
         let contribution = last_idx.checked_mul(stride).ok_or_else(|| {
             ArrowError::InvalidArgumentError(format!(
                 "max addressable offset on axis {k} overflows i64"
@@ -285,7 +283,7 @@ fn check_view_buffer_bounds(
 /// surface the failure.
 fn materialize_strided(
     buffer: &[u8],
-    visible_shape: &[u64],
+    visible_shape: &[i64],
     byte_strides: &[i64],
     byte_offset: i64,
     dtype_size: usize,
@@ -302,7 +300,7 @@ fn materialize_strided(
     // we'd otherwise repeat here, so this walk uses plain arithmetic and
     // unchecked slice indexing within the buffer.
     let ndim = visible_shape.len();
-    let total: u64 = visible_shape.iter().product();
+    let total: i64 = visible_shape.iter().product();
     if total == 0 {
         return Vec::new();
     }
@@ -322,13 +320,13 @@ fn materialize_strided(
 
     // Precompute a small index vector for outer axes (everything except
     // the innermost). For 1D this is empty and we run a single pass.
-    let mut outer_idx = vec![0u64; ndim.saturating_sub(1)];
+    let mut outer_idx = vec![0i64; ndim.saturating_sub(1)];
     loop {
         // Compute the byte offset of the row's first element from the
         // current outer index combination.
         let mut row_off = base;
         for (k, &i) in outer_idx.iter().enumerate() {
-            row_off += (i as i64) * byte_strides[k];
+            row_off += i * byte_strides[k];
         }
 
         if row_bytes_contiguous {
@@ -435,18 +433,37 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
         // Read source shape slice.
         let ss_start = self.band_source_shape_list.value_offsets()[band_row] as usize;
         let ss_end = self.band_source_shape_list.value_offsets()[band_row + 1] as usize;
-        let source_shape: &[u64] = &self.band_source_shape_values.values()[ss_start..ss_end];
+        let source_shape_u64: &[u64] = &self.band_source_shape_values.values()[ss_start..ss_end];
 
         // Reject 0-D bands at the read boundary. Schema doesn't forbid them
         // outright but every consumer assumes ndim >= 1.
-        if source_shape.is_empty() {
+        if source_shape_u64.is_empty() {
             return Err(ArrowError::ExternalError(Box::new(
                 sedona_common::sedona_internal_datafusion_err!(
                     "band {band_row} has empty source_shape; ndim must be >= 1"
                 ),
             )));
         }
 
+        // Convert source_shape u64 → i64 once with overflow check. Every
+        // downstream consumer in the view machinery wants i64 (matches
+        // ViewEntry's signed fields and the stride arithmetic). The cast
+        // is fallible only on cosmically large dims (> 2^63); a clean
+        // internal error is better than a wrap that silently passes
+        // later bound checks.
+        let source_shape: Vec<i64> = source_shape_u64
+            .iter()
+            .map(|&s| {
+                i64::try_from(s).map_err(|_| {
+                    ArrowError::ExternalError(Box::new(
+                        sedona_common::sedona_internal_datafusion_err!(
+                            "band {band_row}: source_shape axis {s} exceeds i64::MAX"
+                        ),
+                    ))
+                })
+            })
+            .collect::<Result<_, _>>()?;
+
         // Resolve data type up front; an unknown discriminant is a
         // schema-corruption bug, not user data, so failing the band loudly
         // here is appropriate.
@@ -474,7 +491,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
                     source_axis: i as i64,
                     start: 0,
                     step: 1,
-                    steps: s as i64,
+                    steps: s,
                 })
                 .collect()
         } else {
@@ -492,7 +509,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
 
         // Full validation: length match, source_axis permutation, bounds,
         // and steps >= 0. Anything malformed is schema-level corruption.
-        if let Err(e) = validate_view(&view_entries, source_shape) {
+        if let Err(e) = validate_view(&view_entries, &source_shape) {
             return Err(ArrowError::ExternalError(Box::new(
                 sedona_common::sedona_internal_datafusion_err!(
                     "band {band_row} has malformed view: {e}"
@@ -517,17 +534,14 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
         // C-order byte strides over the source_shape:
         //   source_strides_bytes[k] = dtype_size * Π_{j>k} source_shape[j]
         //
-        // Computed with checked arithmetic so a corrupt source_shape (a u64
-        // that doesn't fit in i64, or a product that overflows) is rejected
-        // here rather than producing a wrapped stride that silently passes
-        // later bound checks.
+        // Computed with checked arithmetic so a corrupt source_shape whose
+        // product overflows i64 is rejected rather than producing a wrapped
+        // stride that silently passes later bound checks.
         let mut source_strides_bytes = vec![0i64; source_shape.len()];
         source_strides_bytes[source_shape.len() - 1] = dtype_size;
         for k in (0..source_shape.len() - 1).rev() {
-            let next_axis = i64::try_from(source_shape[k + 1])
-                .map_err(|_| overflow_err("source_shape axis exceeds i64::MAX"))?;
             source_strides_bytes[k] = source_strides_bytes[k + 1]
-                .checked_mul(next_axis)
+                .checked_mul(source_shape[k + 1])
                 .ok_or_else(|| overflow_err("source-stride product overflows i64"))?;
         }
 
@@ -552,7 +566,7 @@ impl<'a> RasterRef for RasterRefImpl<'a> {
                 .checked_add(start_off)
                 .ok_or_else(|| overflow_err("view offset accumulation overflows i64"))?;
         }
-        let is_identity_view = is_identity_view(&view_entries, source_shape);
+        let is_identity_view = is_identity_view(&view_entries, &source_shape);
         // byte_offset is non-negative by construction (start >= 0,
         // src_stride > 0). Check defensively so a future refactor that
         // breaks the invariant surfaces a clean internal error rather than

rust/sedona-raster/src/builder.rs

@@ -419,7 +419,21 @@ impl RasterBuilder {
             )));
         }
 
-        validate_view(view, source_shape)?;
+        // Convert source_shape u64 → i64 for validation. `start_band_with_view`
+        // is a builder entry point so producers can express a source_shape
+        // that doesn't fit in i64 — reject it here rather than letting it
+        // wrap inside `validate_view`.
+        let source_shape_i64: Vec<i64> = source_shape
+            .iter()
+            .map(|&s| {
+                i64::try_from(s).map_err(|_| {
+                    ArrowError::InvalidArgumentError(format!(
+                        "start_band_with_view: source_shape axis {s} exceeds i64::MAX"
+                    ))
+                })
+            })
+            .collect::<Result<_, _>>()?;
+        validate_view(view, &source_shape_i64)?;
 
         // Write fields.
         match name {
@@ -470,9 +484,16 @@ impl RasterBuilder {
         self.band_data_count_at_start = self.band_data.len();
 
         // finish_raster compares visible shape against spatial_shape.
+        // `visible_shape_from_view` returns Vec<i64>; the entries are
+        // guaranteed non-negative by `validate_view` above, so the cast
+        // to u64 is lossless.
+        let visible_shape_u64: Vec<u64> = visible_shape_from_view(view)
+            .into_iter()
+            .map(|v| v as u64)
+            .collect();
         self.current_raster_bands.push((
             dim_names.iter().map(|s| s.to_string()).collect(),
-            visible_shape_from_view(view),
+            visible_shape_u64,
         ));
 
         Ok(())

rust/sedona-raster/src/traits.rs

@@ -30,6 +30,11 @@ use sedona_schema::raster::BandDataType;
 /// C-order. Consumers that need a flat row-major buffer should use
 /// `BandRef::contiguous_data()` instead.
 ///
+/// `shape` and `offset` are `i64` (not `u64`) to match the surrounding
+/// stride arithmetic (`strides: Vec<i64>` to allow negative steps);
+/// shape elements are always non-negative — `validate_view` enforces
+/// this — and `offset` is non-negative by construction.
+///
 /// Only `buffer` is tied to the producer's lifetime `'a` (it can be tens of
 /// MBs of pixel data and must not be copied). `shape` and `strides` are
 /// owned `Vec`s — they're tiny (ndim ≤ a handful) so an allocation here is
@@ -38,9 +43,9 @@ use sedona_schema::raster::BandDataType;
 #[derive(Debug)]
 pub struct NdBuffer<'a> {
     pub buffer: &'a [u8],
-    pub shape: Vec<u64>,
+    pub shape: Vec<i64>,
     pub strides: Vec<i64>,
-    pub offset: u64,
+    pub offset: i64,
     pub data_type: BandDataType,
 }
 
@@ -476,7 +481,11 @@ pub trait BandRef {
     /// `dim_names` order. Derived from `view`: `[v.steps for v in view]`.
     /// This is what almost all consumers want; use `raw_source_shape()` only
     /// when you need to address into the raw `data` buffer (e.g. FFI).
-    fn shape(&self) -> &[u64];
+    ///
+    /// `i64` (not `u64`) to match the surrounding view-machinery
+    /// arithmetic (strides, offsets); shape elements are always
+    /// non-negative — `validate_view` enforces this.
+    fn shape(&self) -> &[i64];
 
     /// **Internal/FFI-only.** Natural C-order extent of the band's
     /// underlying `data` buffer, indexed by *source* axis (not visible
@@ -489,17 +498,23 @@ pub trait BandRef {
     /// Use this only when you need to index directly into the raw `data`
     /// bytes (e.g. Arrow C Data Interface, numpy zero-copy views) and you
     /// also handle `view()` and the byte-stride layout from `nd_buffer()`.
+    ///
+    /// Returns `&[u64]` because this is a direct slice into the underlying
+    /// Arrow `UInt64Array` column; converting would require an allocation
+    /// per call. Callers that want i64 should cast at the call site.
     fn raw_source_shape(&self) -> &[u64];
 
     /// Per-visible-dimension view entries describing how the band's
     /// visible axes map onto its `source_shape`. `view().len() == ndim()`.
     /// See `ViewEntry` for per-entry semantics.
     fn view(&self) -> &[ViewEntry];
 
-    /// Size of a named dimension (None if doesn't exist)
+    /// Size of a named dimension (None if doesn't exist).
+    /// Cast i64 → u64 is lossless: `shape()` entries are non-negative
+    /// (`validate_view` enforces it).
     fn dim_size(&self, name: &str) -> Option<u64> {
         let idx = self.dim_index(name)?;
-        Some(self.shape()[idx])
+        Some(self.shape()[idx] as u64)
     }
 
     /// Index of a named dimension (None if doesn't exist)
@@ -678,13 +693,10 @@ pub trait BandRef {
 /// `view[k].steps` is the visible extent along view axis `k` after slicing /
 /// broadcasting. Callers should treat the returned shape as authoritative for
 /// the visible region; `source_shape` is only meaningful in conjunction with
-/// the per-entry `source_axis`.
-///
-/// `validate_view` guarantees `steps >= 0`, so the `as u64` cast is lossless
-/// when the input has already been validated. Callers that haven't validated
-/// yet should still call `validate_view` first.
-pub(crate) fn visible_shape_from_view(view: &[ViewEntry]) -> Vec<u64> {
-    view.iter().map(|v| v.steps as u64).collect()
+/// the per-entry `source_axis`. `validate_view` guarantees `steps >= 0`,
+/// so the entries are all non-negative when input has been validated.
+pub(crate) fn visible_shape_from_view(view: &[ViewEntry]) -> Vec<i64> {
+    view.iter().map(|v| v.steps).collect()
 }
 
 /// True iff `view` is the canonical identity over a C-order source buffer:
@@ -698,16 +710,12 @@ pub(crate) fn visible_shape_from_view(view: &[ViewEntry]) -> Vec<u64> {
 ///
 /// Callers should validate the view first; this function checks lengths
 /// defensively but trusts the inputs otherwise.
-pub(crate) fn is_identity_view(view: &[ViewEntry], source_shape: &[u64]) -> bool {
+pub(crate) fn is_identity_view(view: &[ViewEntry], source_shape: &[i64]) -> bool {
     if view.len() != source_shape.len() {
         return false;
     }
     view.iter().enumerate().all(|(k, v)| {
-        v.source_axis == k as i64
-            && v.start == 0
-            && v.step == 1
-            && v.steps >= 0
-            && (v.steps as u64) == source_shape[k]
+        v.source_axis == k as i64 && v.start == 0 && v.step == 1 && v.steps == source_shape[k]
     })
 }
 
@@ -789,7 +797,7 @@ pub(crate) fn compose_view(
 ///
 /// Runs implicitly inside `RasterBuilder::with_view` (writer) and
 /// `RasterRef::band` (reader); external callers don't need to invoke it.
-pub(crate) fn validate_view(view: &[ViewEntry], source_shape: &[u64]) -> Result<(), ArrowError> {
+pub(crate) fn validate_view(view: &[ViewEntry], source_shape: &[i64]) -> Result<(), ArrowError> {
     let ndim = source_shape.len();
     if view.len() != ndim {
         return Err(ArrowError::InvalidArgumentError(format!(
@@ -820,8 +828,14 @@ pub(crate) fn validate_view(view: &[ViewEntry], source_shape: &[u64]) -> Result<
                 v.steps
             )));
         }
+        if source_shape[sa] < 0 {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "source_shape[{sa}] = {} must be >= 0",
+                source_shape[sa]
+            )));
+        }
         if v.steps > 0 {
-            let s = source_shape[sa] as i64;
+            let s = source_shape[sa];
             if v.start < 0 || v.start >= s {
                 return Err(ArrowError::InvalidArgumentError(format!(
                     "view[{k}].start = {} is out of range [0, {s}) for source axis {sa}",
@@ -1319,7 +1333,7 @@ mod tests {
     struct StubBand {
         dim_names: Vec<String>,
         source_shape: Vec<u64>,
-        shape: Vec<u64>,
+        shape: Vec<i64>,
         view: Vec<ViewEntry>,
     }
 
@@ -1330,7 +1344,7 @@ mod tests {
         fn dim_names(&self) -> Vec<&str> {
             self.dim_names.iter().map(String::as_str).collect()
         }
-        fn shape(&self) -> &[u64] {
+        fn shape(&self) -> &[i64] {
             &self.shape
         }
         fn raw_source_shape(&self) -> &[u64] {