Add RS_Min/RS_Max and RS_Min_Agg/RS_Max_Agg raster functions - #1099
Closed
james-willis wants to merge 4 commits into
Closed
Add RS_Min/RS_Max and RS_Min_Agg/RS_Max_Agg raster functions#1099james-willis wants to merge 4 commits into
james-willis wants to merge 4 commits into
Conversation
Native Rust aggregate UDFs computing the pixel-wise min/max raster across a group, sharing one generic accumulator parametrized by a min/max comparator. Nodata-aware: a pixel equal to the nodata sentinel is treated as absent, with the other raster's value winning outright. State is the output raster type itself, so update_batch and merge_batch share one combine implementation. v1 scope: plain Accumulator only (no GroupsAccumulator), single raster argument, requires matching band count/shape/dtype/nodata across a group.
- RS_Min(a, b[, band]) and RS_Max(a, b[, band]): pairwise (non-aggregate) pixel-wise min/max of one band of two rasters, analogous to LEAST/GREATEST. Shares combine logic with RS_Min_Agg/RS_Max_Agg via a new raster_min_max module. - RS_Min_Agg/RS_Max_Agg now take an optional band_index argument (1-based, default 1, same convention as RS_BandPixelType) and operate on that single band rather than requiring all bands across a group to match. - Mark all four functions experimental in their docs (signature/semantics may still change).
james-willis
changed the title
- combine_bytes previously let NaN silently win comparisons on either side (PartialOrd against NaN is always false, so the second operand always won when it held NaN), discarding a valid value from the other side for both Min and Max. Now treats a non-nodata NaN the same way nodata is already treated -- the valid side wins outright, both-NaN stays NaN -- matching np.fmin/np.fmax and xarray's default skipna=True reductions. - RS_Min_Agg/RS_Max_Agg now require a constant band_index across the raw input rows one accumulator sees directly, erroring clearly instead of silently merging pixels selected from different bands when band_index varies row to row. This check does not extend across merge_batch (partial states from other partitions carry no record of which band_index produced them) -- an accepted gap for now. Found via an adversarial review pass; added regression tests for both.
james-willis
marked this pull request as ready for review
July 30, 2026 20:05
The docs build renders each ```sql fence as a single example and errors
on more than one statement per block ("Expected single SQL statement").
Split the two-overload examples into separate fences, matching the
existing RS_BandPixelType-style convention.
james-willis
marked this pull request as draft
July 30, 2026 22:23
Contributor
Author
|
I think I might be missing something important here. I think this method should return a raster collapse along an array of provided dimensions: RS_Min_Agg(raster, ["x", "y",], 1) // assume some 3rd and 4th dim z and tThis will return a 2d non-spatial raster (tensor?) of all averages at each z/t combo. |
Contributor
Author
|
going a different direction on this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Experimental — signatures/semantics may still change.
RS_Min_Agg/RS_Max_Agg: aggregate UDFs computing the pixel-wise min/max of one band across a group of rasters. Optionalband_indexargument (1-based, default 1, same convention asRS_BandPixelType/RS_BandNoDataValue— an out-of-range index is skipped rather than erroring).RS_Min/RS_Max: the pairwise (non-aggregate) equivalent —LEAST/GREATESTfor one band of two rasters. Same optionalband_indexargument.raster_min_max.rs), so the aggregate and scalar surfaces can't drift on what "min/max of two rasters" means.update_batch/merge_batchshare one implementation.v1 scope, called out in the code:
Accumulatoronly for the aggregates — noGroupsAccumulatoryet (DataFusion falls back to one accumulator per group, correct but not the fastest path for many small groups).docs/reference/sql/{rs_min,rs_max,rs_min_agg,rs_max_agg}.qmd, each cross-linking its pairwise/aggregate counterpart.This is the first of a small family of raster statistics aggregates (mean/sum/std/var/median tracked separately); min/max are the easiest to make exactly and cheaply distributable.
Test plan
cargo test -p sedona-raster-functions— 269 passed, no regressionscargo build -p sedona-raster-functionscargo fmt -p sedona-raster-functionscargo clippy -p sedona-raster-functions --all-targets— clean