test(integration): RS_Resample parity between SedonaDB and Sedona Spark - #1219
Merged
Conversation
…f hand-rolling it
Engines agreeing with each other is not proof either is right — every engine returning the same wrong thing passes the parity claim. compare() now takes an optional keyword-only expected= asserted on the subject first (references are anchored transitively): anything assert_result accepts for table results, a DecodedRaster for raster results. Anchor the tests whose expected value is trivially known: the getter test (written nodata and NULL) and all four passing setter tests (the exact seeded pixels, grid, and per-band nodata) — the setter tests would previously have passed if both engines no-opped identically.
Includes the commits of apache#1211 (bbox placement in the raster fixture writers) and converts the anchored tests' explicit grid placement to bbox=. The anchors keep their GDAL transform tuples — that is the decoded representation assert_decoded_equal compares, not a placement choice.
The nearest-neighbour path agrees bit-for-bit across dimension mode, scale mode (including grown-extent border fill), and the reference-raster and grid-snap overloads; those cases are locked with hand-computed expected= anchors. Interpolating algorithms diverge: Sedona Spark really implements only NearestNeighbor, Bilinear, and Bicubic, silently resampling nearest for every other name it accepts, and even the shared Bilinear/Bicubic kernels differ pixel-wise from GDAL's. Divergences are xfail-cataloged. 13 passing tests, 12 xfails. RS_MapAlgebra is out of scope (SedonaDB does not implement it).
Same exactly-one bbox/gdal_transform surface as the write_* helpers, so anchors can state a north-up grid readably. The instance always carries a gdal_transform afterwards (decoders produce transforms and assert_decoded_equal compares them); bbox is construction sugar, derived from the pixel shape with rasterio's from_bounds.
DecodedRaster now takes the same exactly-one bbox/gdal_transform surface as the writers (apache#1211), so the anchors state the same BBOX the fixtures are placed with and the duplicate transform constant goes away.
With DecodedRaster's bbox construction (apache#1211), every extent-preserving resample anchors on the fixture's own BBOX, and the grown/snapped grids state their extents readably instead of as transform tuples.
main's trinary override semantics (apache#1198) changed SedonaDB's RS_SetBandNoDataValue(rast, band, NULL) from propagating the NULL to clearing that band's nodata (probe: a band holding 7.0 comes back cleared while band 2 keeps its 7.0); Sedona Spark still returns a NULL raster. Split the combined NULL-args test: NULL band index still agrees (both NULL), NULL value is xfail-cataloged.
Addresses the _anchor review comment on apache#1218: the standard random raster now has one definition. DecodedRaster.random builds it (seeded pixels on the RANDOM_GRID_* defaults, scalar nodata broadcast per band), DecodedRaster.write_geotiff writes it (GeoTIFF nodata is file-wide, so per-band anchors refuse to write), and write_random_geotiff and DBEngine.create_random_raster_view delegate to them. Anchors are the same object the fixture registers, so they cannot drift apart — the duplicated BANDS/HEIGHT/WIDTH/BBOX constants and the module-local _anchor helper in the parity suites go away.
james-willis
marked this pull request as ready for review
September 2, 2026 20:00
test_rs_raster_out.py becomes test_rs_setbandnodatavalue.py — the parity suite keeps one module per RS_ function.
The python-wheels workflow runs this suite with a reduced dependency set (see apache#1230); both new tests resolve a bbox through rasterio.
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
2 participants
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.
Two follow-ups ride along with the resample module:
DecodedRaster.random/DecodedRaster.write_geotiff— addresses @paleolimbot's_anchorcomment on test(integration): compare() accepts an optional expected anchor #1218 by combining the random-raster constructors: one definition of the standard raster (seeded pixels on theRANDOM_GRID_*defaults), whichwrite_random_geotiffandcreate_random_raster_viewnow delegate to. Anexpected=anchor is the same object the fixture registers, so they cannot drift; the duplicated grid constants and the module-local_anchorhelper are gone.write_geotiffrefuses per-band nodata (GeoTIFF nodata is file-wide), and a round-trip unit test pins the default grid to the historical transform.The next step of the parity roadmap from #1203/#1212: RS_Resample, probed first on both engines, then locked in with anchored
compare()calls where agreement holds andxfail-cataloged where it doesn't. RS_MapAlgebra is out of scope (SedonaDB doesn't implement it; the roadmap docstring is updated accordingly).