james-willis commented on code in PR #975:
URL: https://github.com/apache/sedona-db/pull/975#discussion_r3483108077


##########
rust/sedona-raster-zarr/src/loader.rs:
##########


Review Comment:
   done



##########
rust/sedona-raster-zarr/src/geozarr.rs:
##########
@@ -122,16 +137,139 @@ fn parse_transform(
     // `[origin_x, scale_x, skew_x, origin_y, skew_y, scale_y]`, so reorder:
     //   origin_x = c, scale_x = a, skew_x = b,
     //   origin_y = f, skew_y = d, scale_y = e.
-    let mut aff = [0f64; 6];
-    for (i, v) in arr.iter().enumerate() {
-        aff[i] = v.as_f64().ok_or_else(|| {
-            ArrowError::InvalidArgumentError(format!("spatial:transform[{i}] 
must be a number"))
-        })?;
-    }
-    let [a, b, c, d, e, f] = aff;
+    let [a, b, c, d, e, f] = [arr[0], arr[1], arr[2], arr[3], arr[4], arr[5]];
     Ok(Some([c, a, b, f, d, e]))
 }
 
+/// Parse the optional `spatial:bbox` attribute into `[xmin, ymin, xmax, 
ymax]`.
+/// `None` when absent. The grid shape is *not* read from `spatial:shape`; the
+/// loader supplies the array's own shape to [`derive_transform_from_bbox`].
+///
+/// A malformed `spatial:bbox` (not a 4-element numeric array) is treated as
+/// absent — it warns and returns `None` rather than failing the load, so the
+/// loader can fall back to coordinate arrays, mirroring how a bad coordinate
+/// array is handled.
+fn parse_bbox(attrs: &serde_json::Map<String, serde_json::Value>) -> 
Option<[f64; 4]> {
+    let v = attrs.get("spatial:bbox")?;
+    let bbox: Option<Vec<f64>> = v
+        .as_array()
+        .and_then(|a| a.iter().map(|e| e.as_f64()).collect());
+    match bbox.as_deref() {
+        Some([xmin, ymin, xmax, ymax]) => Some([*xmin, *ymin, *xmax, *ymax]),
+        _ => {
+            log::warn!(
+                "Zarr group has a malformed `spatial:bbox` (expected a 
4-element numeric array \
+                 [xmin, ymin, xmax, ymax]); ignoring it"
+            );
+            None
+        }
+    }
+}
+
+/// Parse the optional `spatial:registration` attribute (a string). `Ok(None)`
+/// when absent; [`derive_transform_from_bbox`] then defaults to `"pixel"`.
+fn parse_registration(
+    attrs: &serde_json::Map<String, serde_json::Value>,
+) -> Result<Option<String>, ArrowError> {
+    match attrs.get("spatial:registration") {
+        Some(v) => Ok(Some(
+            v.as_str()
+                .ok_or_else(|| {
+                    ArrowError::InvalidArgumentError("spatial:registration 
must be a string".into())
+                })?
+                .to_string(),
+        )),
+        None => Ok(None),
+    }
+}
+
+/// Derive a GDAL-order transform from a `spatial:bbox` and the grid's `height`
+/// and `width` — the array's *own* spatial dimensions, not a `spatial:shape`
+/// attribute (which could drift from the real shape).
+///
+/// `bbox` is `[xmin, ymin, xmax, ymax]`. `registration` (default `"pixel"`) 
sets
+/// how the bbox relates to the grid, per the GeoZarr spatial convention
+/// (<https://github.com/zarr-conventions/spatial>): `"pixel"` means the bbox 
is
+/// the grid's outer edge, spanning all `N` cells (`scale = extent / N`) with 
the
+/// top-left corner on the bbox edge `(xmin, ymax)`; `"node"` means the bbox
+/// endpoints are the *centers* of the border cells, so `N` centers span `N - 
1`
+/// intervals (`scale = extent / (N - 1)`) and the corner sits half a cell 
outside
+/// the bbox. `scale_y` is negative (rows increase downward).
+pub(crate) fn derive_transform_from_bbox(

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to