Skip to content

perf(rust/sedona-raster-gdal): RS_Clip follow-ups from review — zero-copy band output, rotated-raster tests, tester CRS engine - #1055

Merged
james-willis merged 5 commits into
apache:mainfrom
james-willis:jw/rs-clip-followups
Jul 17, 2026
Merged

perf(rust/sedona-raster-gdal): RS_Clip follow-ups from review — zero-copy band output, rotated-raster tests, tester CRS engine#1055
james-willis merged 5 commits into
apache:mainfrom
james-willis:jw/rs-clip-followups

Conversation

@james-willis

@james-willis james-willis commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

clean up comments from dewey in #1000

also drive by clean up some tests to use the raster test harness methods

@github-actions
github-actions Bot requested a review from prantogg July 17, 2026 05:43
@james-willis
james-willis force-pushed the jw/rs-clip-followups branch 2 times, most recently from 385600c to fe1610d Compare July 17, 2026 06:07
… and appends it zero-copy

The mask/crop helpers write their plane's bytes into the caller-owned
per-band buffer instead of returning a fresh Vec per plane, and the
finished band bytes move into the output BinaryViewArray as a shared view
block (append_band_data_buffer) instead of being copied through the
builder — one allocation per band, no redundant copies, pinned by a
data_buffers() assertion so a refactor can't quietly reintroduce the copy.

Criterion (local, --save-baseline): Clip(1024x1024, Polygon(large))
704.8µs -> 635.9µs (-9.8%); the micro cases are bounded by per-call GDAL
setup (~17µs) and move less than 0.5µs.
…sterio reference

The crop window maps the geometry envelope's corners through the inverted
affine and the output origin shifts by the full affine (both skew terms),
so skewed and rotated rasters clip correctly — but every existing clip
fixture was north-up. Two parity cases (sheared, and ~30-degree rotated,
crop on and off) pin it against rasterio, whose geometry_window agrees
with the envelope math on rotated grids.
…jecting UDF tests

The tester's ConfigOptions already carry SedonaOptions, but the default
CRS engine errors on use, so a tester-invoked UDF could never exercise a
reprojection path (only config-less direct calls reach the global-PROJ
fallback). with_crs_engine swaps in a real engine; rs_clip demonstrates
the pattern with sedona-proj's LazyProjEngine as a dev-dependency,
pinning both halves: the default engine errors, and the reprojected clip
matches the same-CRS clip through the tester.
@james-willis
james-willis force-pushed the jw/rs-clip-followups branch from fe1610d to 9670b6e Compare July 17, 2026 06:14
…ec helpers

The result-side navigation in the pre-existing clip tests — ScalarValue
destructures into RasterStructArray band-byte chains and raw
column_by_name/downcast walks — becomes assert_raster_scalar_equals /
assert_rasters_equal against declarative RasterSpec expectations. Besides
being readable, the specs assert strictly more: the crs-mismatch test now
pins the exact clip output instead of only 4326==3857 equality, the
band-column regression pins both rows' contents instead of mere
inequality, and band-0 pins both bands' values rather than a band count.
@james-willis
james-willis requested review from paleolimbot and removed request for prantogg July 17, 2026 06:57
@james-willis
james-willis marked this pull request as ready for review July 17, 2026 06:58

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Comment thread python/sedonadb/tests/functions/test_rs_clip.py Outdated
Comment thread python/sedonadb/tests/functions/test_rs_clip.py Outdated
Comment thread rust/sedona-raster-gdal/src/rs_clip.rs Outdated
@james-willis
james-willis merged commit fe68723 into apache:main Jul 17, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants