test(integration): compare() accepts an optional expected anchor - #1212
Closed
james-willis wants to merge 8 commits into
Closed
test(integration): compare() accepts an optional expected anchor#1212james-willis wants to merge 8 commits into
james-willis wants to merge 8 commits into
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.
james-willis
marked this pull request as draft
September 1, 2026 21:51
Includes the commits of #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.
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 (#1211), so the anchors state the same BBOX the fixtures are placed with and the duplicate transform constant goes away.
Contributor
Author
|
Re-raised from the fork as #1218 (contributor branches shouldn't live in the Apache repo). Same commits; the apache branch is deleted. |
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.
Stacked on #1211 (its branch lives on a fork, so the base can't be retargeted onto it — its commits are merged in instead; this diff deduplicates when #1211 lands. Merge #1211 first.)
Implements @paleolimbot's post-merge suggestion on #1203: an optional parameter on
compare()that lets the reference value be specified, guarding the parity claim against vacuous agreement — every engine returning the same wrong thing passes a pure engine-vs-engine comparison.The API
expectedis keyword-only and optional. The subject is asserted against the anchor first (clearest failure when all engines agree on a wrong value), then against each reference as before — so the references are anchored transitively. Parity-only remains the default for cases where the reference is hard to state.Coverage added
Anchors on every test whose expected value is trivially known:
test_rs_band_nodata: the written nodata for both bands, and[(None,)]for the no-nodata fixture.test_rs_setbandnodata,_band2,_overwrite,_two_arg_single_band): the fullDecodedRaster— the exact seeded pixels (reconstructible becauserandom_raster_datais seeded), the grid (stated explicitly so the anchor and the fixture can't drift apart), and the per-band nodata.The anchored tests place their grid with
bbox=, and the anchors state the same BBOX viaDecodedRaster's bbox construction (added to #1211) — the instance still carries the derivedgdal_transform, which is whatassert_decoded_equalcompares.The setter anchors close a real hole: with parity only, both engines no-opping identically (source nodata unchanged) would have passed every setter test.
Verification
21 passed, 13 xfailedlocally (pyspark 4.0.4, Sedona 1.9.1), plus chore(python/sedonadb): accept a bbox in create_random_raster_view #1211'stest_raster_testing.py(26 passed) in-branch.AssertionError; the correct anchors pass.This suite is not yet wired into CI (#1206 does that), so the counts above are from a local run.