paleolimbot commented on code in PR #975:
URL: https://github.com/apache/sedona-db/pull/975#discussion_r3468617242
##########
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:
While you're here, can you move this function to sedona-raster (e.g.,
`AffineMatrix::from_bbox_and_spatial_shape()` or something). It would be
especially nice to support this in `Raster.from_numpy()` (probably easier to do
it in this PR but OK if you want it to be a follow-up). A bonus of doing that
would be that you can do a sanity check against a test fixture (ask rasterio
for the bounds of something and check its affine matrix).
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
Review Comment:
Not essential, but this free function is now a few hundred lines long and
would benefit from being split (`try_from_bbox()`, `try_from_transform()`)
--
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]
