Claude/py03 bindings implementation 6wrqtq - #6
Merged
Conversation
The library is 266,000 lines: about 4,100 free functions, 2,200 methods,
336 structs and 88 enums across 71 top-level modules. Writing a Python
binding for that by hand is not the hard part -- keeping it in step with
the library afterwards is, and that failure is silent. Nothing breaks
when a function is added and the binding is not regenerated; the function
simply is not there, and nobody finds out until someone goes looking for
it.
So the bindings are derived instead. `rustscan.py` reads the crate's
public API out of the source -- a scanner that knows where the code is,
so a brace inside a doctest cannot unbalance an item and a `;` inside
`[f64; 6]` cannot end a declaration. `generate.py` decides how each item
crosses into Python and writes the wrapper. CI re-runs the generator and
fails if what is committed differs, which is the only thing that keeps
generated code honest.
What crosses over: 4,070 of 4,149 free functions, 2,230 of 2,277 methods,
416 of 426 types and every constant, across 296 Python modules mirroring
the Rust tree. COVERAGE.md lists every item that did not, by name, with
the reason -- a generic parameter that cannot be monomorphised, a
`&dyn Trait` with no Python equivalent, a routine returning a closure.
Four things are translated rather than transcribed, so the result reads
as Python rather than as Rust seen through glass:
- `Result::Err` becomes an exception, under one `PhysicsError` root,
with the data-carrying variants carrying their data onto the
exception. The library also validates arguments with `assert!`,
which is right for Rust and would abort the interpreter here; a
panic guard turns those into `InvalidArgumentError` holding the
assertion's own message.
- A `Vec3` argument accepts `(x, y, z)`, a `Matrix` accepts a list of
rows, and the wrapper classes keep their methods, operators,
indexing and `tolist()`.
- `Complex`, `BigInt` and `Rational` have exact Python counterparts,
so they are translated to `complex`, `int` and `fractions.Fraction`
rather than wrapped. Their methods become functions in the module
that defines the type.
- Anywhere the library takes a `&dyn Fn`, a Python callable will do,
and an exception raised inside it comes back out with its own
traceback instead of turning into a NaN.
The GIL is released around calls that take or return arrays and held for
scalar ones, where releasing it would cost more than the call.
`.pyi` stubs are generated alongside, and `check_stubs.py` imports the
built extension and compares it against them name by name -- a stub that
disagrees with its module is worse than no stub, because it type-checks
code that will fail at run time.
The crate is a workspace of its own. The library's Cargo.lock still holds
exactly one package, and `cargo build` at the repository root is
unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
Five kinds of signature were being skipped or, worse, bound in a way that
compiled and did nothing.
- `&mut [f64]` and `&mut [Vec2]` are outputs written through an
argument. The values now go back into the list they came from, and a
tuple -- which cannot receive them -- is a TypeError rather than a
silent no-op.
- `&mut self -> &mut Self` is Rust's chaining idiom. The wrapper hands
the same Python object back, so `c.h(0).cx(0, 1)` reads as it does in
Rust; returning a copy would have thrown away every gate after the
first.
- `&T` where `T` is not `Clone` is passed as a borrow of the wrapper
rather than not at all.
- `new` returning `Result<Self, E>` is a constructor. Missing that left
a third of the classes unconstructible from Python.
- `impl Iterator<Item = T>` returns come back as a list. Every such
routine here is finite by construction, so this changes laziness and
not termination.
`special::gamma` is both a module and, through a re-export, a function.
Rust keeps types, values and modules in separate namespaces; Python does
not, and the re-export was overwriting the module -- taking
`special.gamma.gamma_p` and everything beside it with it. The module now
wins, the function stays reachable one level down, and COVERAGE.md says
which nine names this affects and where to find each one.
Coverage: 4,086 of 4,149 free functions and 2,254 of 2,277 methods, up
from 4,028 and 2,232.
The suite that goes with it is 96 tests. They are not a second opinion on
the library's mathematics, which it checks thoroughly itself; they check
the crossing. That arguments arrive in the order and units the Rust
function expects. That an exception raised inside a Python integrand
comes back out with its own traceback rather than becoming a NaN. That an
in-place argument really is written back. That results stay correct with
eight threads in the library at once, and that a panic on one thread does
not disturb another. And a sweep over the whole surface -- every module
attached under the right parent, every function carrying a docstring and
a signature, no name shadowed by a submodule -- because a registration
bug shows up there and in no hand-written test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
`guarded` bundled the panic guard with its error conversion; the generator emits the two separately, so nothing ever called it. The bindings now build with no warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
The wheel-building job ran on macOS and Windows as well as ubuntu. macOS runners bill at ten times the ubuntu rate and Windows at twice, and building the same crate on three platforms establishes nothing the ubuntu job has not. Gating it on tags did not make it free -- it made it expensive later, on the day someone cuts a release and is not thinking about minutes. Removed; when there is a release to cut, the wheels can be built then. The Python matrix goes too. The extension is an abi3 wheel, so one binary serves 3.9 through 3.13 by construction; compiling it a second time under 3.13 tested the same bytes. Also scopes the `*.so` ignore to where `maturin develop` actually drops the extension, rather than ignoring every shared object in the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
Restores the per-platform wheel build, in a workflow of its own rather
than as a job inside python.yml. That separation is the substance of the
change, not tidiness: python.yml runs on every push and every pull
request, and this file has one trigger, `push: tags`, with no `branches`
key and no `pull_request` key at all. A branch push cannot reach a macOS
runner from here -- not because a condition forbids it and could be
edited away, but because no event is wired to it.
macOS bills at ten times the ubuntu rate and Windows at twice, so the
choice of targets is about cost as much as coverage:
- macOS builds universal2, one wheel covering Apple Silicon and Intel,
so it is one mac runner per release rather than two.
- Linux builds inside a manylinux container. Plain `maturin build` on
the runner produces a manylinux_2_35 tag pinned to whatever glibc
that image happens to carry, which will not install on an older
distribution.
Each job also carries an `if:` on the tag ref. It is redundant with the
trigger and deliberately so: anyone who later adds a second event to this
file has to walk past it before these runners start answering to it.
No sdist. The crate depends on the parent by path, so an archive rooted
at bindings/python would not carry the library it binds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
Turns the wheel build into an actual release: a source distribution and three wheels, uploaded to PyPI by Trusted Publishing. Still one trigger, `push: tags`, and still no `branches` or `pull_request` key, so the macOS and Windows runners remain unreachable from a branch push. The sdist is the correction. I said earlier there could not be one, because the bindings depend on the library by path and an archive rooted at bindings/python would not carry it. That was wrong: maturin vendors the path dependency, and the archive contains the whole crate -- src/, Cargo.toml, LICENSE -- with cargo resolving `path = "../.."` inside the extracted tree. Without it the package would be uninstallable on every platform the three wheels do not cover. Publishing uses OIDC rather than an API token, so there is no long-lived secret in this repository. `permissions:` is empty at the top of the file and the publish job grants itself `id-token: write` and nothing else. It runs in an Environment named `pypi`, which is what PyPI is told to trust and is also where a required reviewer can be attached. A version check runs first, before any runner minutes are spent, and again on every pull request in python.yml. PyPI does not allow a version to be re-uploaded even after deleting it, so tagging v0.2.0 against a Cargo.toml that still says 0.1.0 is a mistake with no undo. The check compares the tag, the crate and the bindings, which move together because the Python package is the same library. `pip install rust-physics-engine` is now the install line in both READMEs; the name is unclaimed on PyPI. The workflow header says what to enter on pypi.org before the first release, since none of this works until that is done once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
Matches the trusted publisher registered on PyPI, which names the project `numeria`, this repository, the workflow file `python-release.yml` and the `pypi` environment. All four have to keep agreeing or the upload is refused, so the workflow header now lists them together rather than describing a setup step that is already done. The import name moves with the distribution name. `pip install numeria` giving you `import rust_physics_engine` is the kind of mismatch that only exists in packages old enough to have been stuck with it, and there is no reason to start a new one that way. So the top-level name is `numeria` and the tree beneath it is unchanged: `rust_physics_engine::linalg::lu::solve` is `numeria.linalg.lu.solve`, as it was before under a different root. The Rust crate keeps its name. Only the Python surface moves: the distribution, the package directory, the `module` attribute on every generated class, the exceptions' `__module__`, the stubs, and the tests. Nearly all of it follows from one constant in the generator, which is what made a rename of this size a five-minute change rather than a weekend. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
The rename matched `rpe.` with a trailing dot, which left the one place the alias is used bare -- `hasattr(rpe, name)` -- and a docstring in the generator. Caught by the suite rather than by reading, which is the point of having it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QzknpXr8HMpHXU6Xzv9UDV
There was a problem hiding this comment.
🟡 Changes recommended
The generated .pyi stubs currently include at least one syntax error and several missing type imports (e.g., MutableSequence, Mat4, RayHit) which will break stub parsing/type-checking and likely fail the new CI stub verification step.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a full PyO3-based Python binding layer for the rust_physics_engine crate, published as the numeria Python package, including generated Rust wrapper modules/types, generated .pyi stubs, and CI to ensure the generated bindings stay in sync with the Rust source.
Changes:
- Introduces the Python extension module entry point (
bindings/python/src/lib.rs) plus hand-written runtime support for error mapping, coercions, and Python-callback bridging. - Adds a large set of generated Rust binding modules/types under
bindings/python/src/generated/**. - Adds the Python packaging layout (
pyproject.toml, pure-Pythonnumeria/__init__.py, type stubs) and a dedicated GitHub Actions workflow for building/testing the bindings.
File summaries
| File | Description |
|---|---|
| README.md | Documents Python availability on PyPI (numeria) with examples and binding behavior notes. |
| bindings/python/Cargo.toml | Defines the standalone Rust workspace/crate for the PyO3 extension module. |
| bindings/python/pyproject.toml | Configures maturin build + Python package metadata for numeria. |
| bindings/python/check_version.py | Adds a safety check ensuring crate/bindings/tag versions agree before release. |
| bindings/python/src/lib.rs | Implements the PyO3 module entry point (numeria._core) and registers generated submodules/errors. |
| bindings/python/src/runtime/mod.rs | Runtime module root for the hand-written binding support surface. |
| bindings/python/src/runtime/callback.rs | Implements Python-callable → &dyn Fn adapter with deferred exception re-raise. |
| bindings/python/src/runtime/errors.rs | Defines the Python exception hierarchy and panic→exception guarding/mapping. |
| bindings/python/src/generated/** | Generated Rust wrappers and registration code for bound functions/types/modules. |
| bindings/python/python/numeria/init.py | Pure-Python package initializer that installs submodules into sys.modules. |
| bindings/python/python/numeria/**/*.pyi | Generated type stubs for the Python-facing API surface. |
| bindings/python/python/numeria/py.typed | Marks the package as typed for PEP 561 tooling. |
| .github/workflows/python.yml | Adds CI job to regenerate-check, version-check, build/install, run tests, and verify stubs. |
| .gitignore | Ignores Python-binding build artifacts (with one path needing correction). |
Review details
- Files reviewed: 91/661 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+6
to
+8
| from collections.abc import Callable, Sequence | ||
| from fractions import Fraction | ||
| from typing import Any, Optional |
| from typing import Any, Optional | ||
|
|
||
| from numeria.quaternion import Quaternion | ||
| from numeria.math import Vec3 |
Comment on lines
+10
to
+14
| from numeria.spatial.primitives import Aabb | ||
| from numeria.spatial.primitives import Ray | ||
| from numeria.spatial.primitives import Sphere | ||
| from numeria.spatial.primitives import Triangle | ||
| from numeria.math import Vec3 |
|
|
||
| from . import analyze, generate, isosurface, parameterize, subdivide, surfaces | ||
| from numeria.spatial.primitives import Aabb | ||
| from numeria.spatial.bvh import Bvh |
|
|
||
| Rust: `geometry::mesh::Mesh` | ||
| """ | ||
| def __init__(self, ) -> None: ... |
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
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.
No description provided.