Skip to content

ci(python/sedonadb-geopandas): run tests, build wheels, and verify in releases - #1095

Merged
jiayuasu merged 2 commits into
apache:mainfrom
jiayuasu:feature/geopandas-package-ci
Aug 7, 2026
Merged

ci(python/sedonadb-geopandas): run tests, build wheels, and verify in releases#1095
jiayuasu merged 2 commits into
apache:mainfrom
jiayuasu:feature/geopandas-package-ci

Conversation

@jiayuasu

Copy link
Copy Markdown
Member

sedonadb-geopandas (added in #1052) is not referenced by any workflow, so its tests never run upstream and it has no path to release: the main pytest invocation runs inside python/sedonadb and does not collect them, and there is no wheel build. This wires the package in the same way the other Python packages are wired, and fixes two packaging problems found along the way.

Python CI

Installs the package (after sedonadb and sedonadb-expr, so pip resolves both from source rather than PyPI) and runs its 23 tests, mirroring the existing sedonadb-expr steps.

Wheels

wheels-geopandas.yml, modelled on wheels-expr.yml: build a universal wheel and sdist, test the built wheel, then publish nightly to Gemfury. It downloads both the sedonadb and the sedonadb-expr wheel artifacts, because the .geo accessor this package delegates to comes from sedonadb-expr. Invoked from python-wheels.yml with needs: ["expr"] (which transitively waits on the core wheel jobs), plus the corresponding paths trigger.

Release verification

verify-release-candidate.sh now installs and tests this package alongside sedonadb-expr and sedonadb-zarr. Without this it would ship in the release tarball unverified.

Fix: version was hard-coded

The package declared version = "0.4.0" literally, while the sibling Python packages derive their version from the workspace Cargo.toml. Two consequences: every nightly wheel would publish as the same 0.4.0, and the value would silently drift from the workspace at each release bump. It now uses the same _version.py + [tool.hatch.version] mechanism as sedonadb-expr.

Verified end to end: ci/scripts/set_dev_version.py rewrites the workspace version to 0.4.0-alpha62, and the resulting wheel is sedonadb_geopandas-0.4.0a62-py3-none-any.whl.

Fix: dependency floors did not admit nightlies

Dependencies were unpinned (sedonadb, sedonadb-expr). Nightly builds version as 0.4.0aN, which is PEP 440-less than 0.4.0, so a plain >=0.4.0 floor would reject the matching nightly wheel and break pip install of the nightly for users, not just CI — the same trap documented in sedonadb-zarr. The floors are now explicit prereleases: sedonadb>=0.4.0a0 and sedonadb-expr>=0.4.0a0.

Confirmed: 0.4.0a62 satisfies >=0.4.0a0 and is rejected by >=0.4.0.

Not included

packaging.yml, which installs packages so mkdocstrings can build generated API-reference pages. This package has no such page, so adding it there would have no effect.

Verification

Wheel and sdist build; wheel metadata carries the intended Requires-Dist and extras; both workflow files parse and the job graph resolves as expected; bash -n passes on the release script; ruff format / ruff check clean; the package's 23 tests pass via the exact commands CI runs.

@github-actions
github-actions Bot requested a review from prantogg July 29, 2026 09:13
@jiayuasu

Copy link
Copy Markdown
Member Author

For anyone looking at the failing checks here: the python-wheels failures (linux-arm64, linux-x86_64, macOS-arm64) are pre-existing on main and not caused by this PR — every python-wheels run on main has failed the same way for several days, on test_rs_reprojectmatch.py::test_reproject_match_null_raster_is_null (ModuleNotFoundError: No module named 'rasterio', 1 failed / 3143 passed). #1096 fixes that with the missing pytest.importorskip, which should turn these green here too.

This PR shows them for the first time only because it touches python-wheels.yml, which is in that workflow's pull_request paths filter.

The job that exercises this PR's change — python ubuntu-latest — passes, including the new Install sedonadb-geopandas and Run tests (sedonadb-geopandas) steps.

@jiayuasu
jiayuasu force-pushed the feature/geopandas-package-ci branch from 7aa58c5 to a813c43 Compare July 29, 2026 18:56
@jiayuasu

Copy link
Copy Markdown
Member Author

Status note on the checks here, since the remaining red one is not from this PR.

All four Linux/macOS wheel jobs now pass after rebasing onto #1096 (linux-x86_64, linux-arm64, macOS-amd64, macOS-arm64), as does python ubuntu-latest, which is the job that actually exercises this PR's change — including the new Install sedonadb-geopandas and Run tests (sedonadb-geopandas) steps.

windows-x86_64 still fails, for a different and pre-existing reason. #1096 did fix the rasterio test failure on Windows too; clearing it revealed a second problem underneath that the earlier failure had masked:

Run Windows result
main, before #1096 pytest ... failed with code 1 (the rasterio test failure)
this PR, after #1096 3210 passed, 2452 skipped, then pytest ... failed with code 3221226505

Every test passes and then the process exits with 3221226505 = 0xC0000409 (STATUS_STACK_BUFFER_OVERRUN). The last lines of the log are the final test passing and the summary line, so this looks like a crash during interpreter teardown rather than a test failure — plausibly native-side shutdown (DLL unload ordering or async runtime teardown), though that is a guess from the log alone.

Worth flagging because the impact is wider than this PR: expr is gated on all four core wheel jobs, so while windows-x86_64 fails, the expr, zarr, and (new here) geopandas jobs are all skipped, and the nightly extension wheels never publish.

One consequence for reviewing this PR: wheels-geopandas.yml has therefore never executed — it was skipped both before and after the rebase. Its wiring has been checked statically (artifact names match their producers in wheels-linux and wheels-expr, both download into a single prebuilt/ so one --find-links resolves both, and the >=0.4.0a0 floors let the prerelease wheels satisfy the package's dependencies), and the build itself was verified locally (set_dev_version.py0.4.0-alpha62sedonadb_geopandas-0.4.0a62-py3-none-any.whl). But the download/install/test sequence in that workflow has not run in CI yet, and will not until the Windows job is green. Happy to leave this as a draft until then if that is preferred.

I have not filed an issue for the Windows crash, since the native/Windows side is better judged by whoever owns it — glad to open one with this evidence if that is useful.

@jiayuasu
jiayuasu force-pushed the feature/geopandas-package-ci branch from a813c43 to 0592355 Compare August 7, 2026 05:03
@jiayuasu
jiayuasu marked this pull request as ready for review August 7, 2026 06:06
@jiayuasu

jiayuasu commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Taking this out of draft. Status update, since the situation changed with the recent wheel-build work and my earlier comment above is now out of date.

Rebased onto current main, which includes #1100 and #1111. #1111 is the relevant one here: adding if: ${{ !cancelled() }} to the extension jobs means a failing platform no longer skips them, so the new geopandas job is finally reachable and wheels-geopandas.yml is being exercised for the first time in the run on this commit.

On the gating: the geopandas job deliberately keeps a plain needs: ["expr"] rather than also taking if: ${{ !cancelled() }}. It downloads the release-expr-wheels artifact, and wheels-expr.yml uploads that artifact after its test step — so if expr fails there is no artifact to download, and running anyway would just produce a confusing download failure. Requiring expr to succeed is the accurate condition for this job; the !cancelled() on expr itself is what decouples it from a failing platform.

Two notes for review:

  • python ubuntu-latest passes, including the new Install sedonadb-geopandas and Run tests (sedonadb-geopandas) steps. Any red wheel-platform checks here are wheel-build issues unrelated to this PR and are being addressed separately, so this PR is not trying to fix them.
  • The workspace version moved from 0.4.0 to 0.5.0 while this was open, which incidentally exercised the versioning change in this PR: because the package now derives its version from the workspace Cargo.toml, it reports 0.5.0 automatically. The hard-coded version = "0.4.0" it replaced would be wrong as of that bump. The dependency floors stay at >=0.4.0a0, since they record the minimum version providing the .geo accessor and DataFrame.unnest, and 0.5.0 satisfies them.

@jiayuasu

jiayuasu commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

@james-willis @paleolimbot can you take a look and approve if LG? This will unblock me to add more geopandas functions

@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! We'll keep an eye on the build for this...I'm hoping to sort out all of the wheel failures next week (James has a PR that may fix the Windows one).

While it's strictly correct to always test sedonadb-geopandas after sedonadb and sedonadb-expr like you've done it here, for the most part you can use any sedonadb >= 0.4. As long as you're a tiny bit careful with your implementation (e.g., if hasattr(df, "unnest"), you could have a separate workflow for sedonadb-geopandas installing from nightly, which will probably run in less than a minute (with your current set up you'll have to wait a while to get feedback from CI specifically for sedonadb-geopandas). You can keep the duplicate coupled check to make sure we don't break sedonadb-geopandas with changes to sedonadb. Your call!

Comment on lines +29 to +35
# The `.geo` accessor and `DataFrame.unnest` this package builds on landed in
# 0.4.0. The floor is an explicit prerelease (`0.4.0a0`) so dev/nightly
# builds — which version as `0.4.0aN` and are PEP 440-*less* than `0.4.0` —
# satisfy it; a plain `>=0.4.0` would reject the matching nightly wheels
# (and break `pip install` of the nightly for users, not just CI).
"sedonadb>=0.4.0a0",
"sedonadb-expr>=0.4.0a0",

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.

Probably >=0.4.0 is sufficient here since both are now on PyPI

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed to >=0.4.0, thanks — that is simpler and it does hold now.

Recording the reasoning, since the two facts look contradictory at first: the prerelease floor was doing real work when it was written, because the workspace was on 0.4.0 at the time, so nightlies versioned as 0.4.0aN, and PEP 440 orders 0.4.0a62 below 0.4.0 — a plain >=0.4.0 rejected exactly the nightly wheel it was paired with (the same trap sedonadb-zarr documents). The workspace has since moved to 0.5.0, so nightlies are 0.5.0aN, which satisfies >=0.4.0 fine, and that stays true for later cycles.

The condition is therefore "the floor must be a released version, not the version currently being developed", so I kept a short comment to that effect rather than the prerelease pin — otherwise someone raising the floor to match the dev line would silently reintroduce it.

@jiayuasu
jiayuasu force-pushed the feature/geopandas-package-ci branch from 0592355 to 713adaa Compare August 7, 2026 17:49
@jiayuasu

jiayuasu commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Correction to my comment above: I claimed the geopandas job should keep a plain needs: ["expr"] and that adding if: ${{ !cancelled() }} would be wrong. The first half was wrong, and the run on the previous commit demonstrated it.

What happened: macOS-amd64 was cancelled after running long. expr ran anyway thanks to its !cancelled() and expr-build succeeded — but geopandas was skipped, because a cancelled job upstream in the graph skips dependents that have not opted in. So the plain needs did not just require expr to succeed, it also inherited the cancellation.

The part of my reasoning that was sound is that this job cannot simply copy !cancelled() either: it downloads the release-expr-wheels artifact, and wheels-expr.yml uploads that after its test step, so running when expr failed would only produce a confusing "artifact not found" failure rather than a clean skip. The condition now states both requirements explicitly:

if: ${{ !cancelled() && needs.expr.result == 'success' }}

Do not skip merely because something upstream was cancelled, but do require expr to have succeeded.

Worth noting this is the one class of bug that could not be caught by reading the workflow — it needed the job to actually be reached in a run with a cancelled platform, which is why it was worth waiting for an execution rather than merging on the strength of the wiring alone. It still has not run end to end; that remains pending on the wheel builds being stable enough for the chain to complete.

@jiayuasu
jiayuasu merged commit 5e3007b into apache:main Aug 7, 2026
17 of 21 checks passed
@james-willis

james-willis commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This seems to have broken the CI of main. The failure shows in the latest commit here. @jiayuasu can you please take a look?

https://github.com/apache/sedona-db/actions/runs/31204113817/job/92977392654

@jiayuasu

jiayuasu commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Thanks for catching this, and sorry for the breakage. Fix is up in #1134.

Cause: the geopandas / geopandas-build job ran for the first time after this merged — before that it was always skipped, because the extension jobs were gated behind platform jobs that were failing — and its test step needs geoarrow-pyarrow, which was not being installed. GeoPandas interop in this package goes through sedonadb's pandas conversion, which imports geoarrow.pyarrow, so every test that materializes a frame failed with No module named 'geoarrow'.

It was missing in two places: the package's geopandas/test extras (so pip install sedonadb-geopandas[geopandas] was broken for users too, not just CI), and the wheels job, which listed test dependencies inline and installed the bare wheel, so it never read the extras. #1134 fixes both, and has the wheels job install the wheel with its [test] extra so the list has a single home.

It stayed hidden because both the dev environment and the python.yml job install sedonadb[test], which happens to provide geoarrow-pyarrow — only a clean environment exposes it. #1134 also adds a small nightly-based workflow that runs the package's tests in exactly such an environment in about a minute, so this class of problem surfaces before merge rather than after.

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.

3 participants