Add python bindings for bonsai#60
Conversation
|
Again, great work on picking up items for the issues tab. This looks like the right direction 🚀 Its gonna take me some time to review all of this, and I'm gonna have to do it in portions |
| while True: | ||
| await asyncio.sleep(0.5) | ||
| now = time.perf_counter() |
There was a problem hiding this comment.
why the async sleep here? threading and queue does not rely on async
There was a problem hiding this comment.
Just a simple example of showing an async job happening on a different thread, What would you recommend?
There was a problem hiding this comment.
await asyncio.sleep(0.5) is identical to time.sleep(0.5) here. The threads progress regardless because they're OS threads driven by the kernel scheduler, not by the asyncio loop. So this example is threaded and not async
I think the simplest here is probably to rename the file to threaded_drone.py and drop asyncio and use time.sleep(0.5) instead. And then you can introduce another example called async_drone.py that demonstrate another concurrency model replacing the threading jobs with coroutines and asyncio.Queue. Then we'll demonstrate both cases for our users, e.g my actions block on hardware → look at threaded; my actions hit a network API → look at async
| [project] | ||
| name = "bonsai-py" | ||
| description = "Behavior trees in Python, powered by the bonsai-bt Rust crate." | ||
| readme = "README.md" |
There was a problem hiding this comment.
I think the directory bonsai-py is good to separate the Rust implementation from the python implementation, however, I think the exported library in PyPi should be bonsai-bt and not bonsai-py as its already implicit that its a python library. So import in python would be import bonsai_bt instead
There was a problem hiding this comment.
I kept all names the same to prevent confusion during dev (having names in code/file paths be different than the name of the published version can get tricky). I will think more about this if you have a strong preference. I can see your point, just need to think the tradeoffs for future dev confusions.
There was a problem hiding this comment.
Renamed after much thought. The python package will also be bonsai-bt now, however in our source code it will live under bonsai_py folder. I saw that other packages follow same strategy. The argument really came down to whether the burden should be created for the developers, or for the users. Keeping different names is better for dev but not for users, while keeping same name is much more convenient for the user.
| license = { text = "MIT" } | ||
| requires-python = ">=3.10" | ||
| authors = [ | ||
| { name = "Kristoffer Solberg Rakstad", email = "kristoffer.solberg@cognite.com" }, |
There was a problem hiding this comment.
I think I've included my work email by mistake in the past, but use my personal email here instead: solkristoffer@gmail.com
There was a problem hiding this comment.
Also, when creating the PyPi account, are you able to create dual ownership of that account with the two of us?
There was a problem hiding this comment.
done. should I go ahead and do a find-replace everywhere for this email? Its in three other places.
We will both need to create PyPI accounts. The steps look like this:
After the first successful publish:
- You will create your own PyPI account at https://pypi.org/account/register/ . Then give me your PyPI username.
- On my side, go to https://pypi.org/manage/project/bonsai-py/collaboration/.
- I will enter your PyPI username and pick role Owner .
- PyPI emails you an invite. From that point on, both are Owners on PyPI.
A key info is that neither of us will actually have the authority to publish a new version. Only this repository will have the rights. The owner tags are simply "These are the emails to reach out for questions". The only single way to publish a new version will be to run the github action in this repository.
There was a problem hiding this comment.
done. should I go ahead and do a find-replace everywhere for this email? Its in three other places.
ye, you can do that, thanks
There was a problem hiding this comment.
The only single way to publish a new version will be to run the github action in this repository.
Will the python package build from source, meaning that we can keep the Rust and Python releases separate? Its not like we have to publish the Rust version first, then the python version after. That reminds me I should set up a workflow for releasing Rust packages automatically after version bump, I've just done that manually until now
There was a problem hiding this comment.
Will the python package build from source, meaning that we can keep the Rust and Python releases separate? Its not like we have to publish the Rust version first, then the python version after.
Correct, they are separate. We can publish either/both based on preference.
I should set up a workflow for releasing Rust packages automatically after version bump
Created issue #67
| r""" | ||
| Verification script for bonsai-py. | ||
|
|
||
| WHAT THIS IS | ||
| Python-side assertions for the installed `bonsai_py` extension module. | ||
| Every test runs on every invocation; the script exits 0 on full pass, | ||
| 1 on the first failure, 2 on a usage error. |
There was a problem hiding this comment.
Great addition, is this best-practices for py03 to include a verification script?
There was a problem hiding this comment.
this actually had a lot o repeated tests that pytest already covered. I refactored the tests so there's less duplicates now.
But yes, its generally good practice to test every single API surface, to catch drifts when the source code changes without updating the python bindings. And its only possible to do this if both the source and the bindings live in the same repo, otherwise catching drifts become impossible and a headache honestly.
Sollimann
left a comment
There was a problem hiding this comment.
what is the py.typed file for?
Sollimann
left a comment
There was a problem hiding this comment.
I think we should add a note under https://github.com/Sollimann/bonsai#using-bonsai that we support python too and e.g link to the bonsai-py folder for more info. Also, In the bonsai-py folder mabye include a super simple example showing a side-by-side view of a very simple BT in Rust and then the same one in python. WDYT ?
| # bonsai-py examples | ||
|
|
||
| Pure-Python examples mirroring `examples/` in the Rust workspace. Each example is a single self-contained `.py` file. |
There was a problem hiding this comment.
can we done in a separate PR, but I think it would be nice to have a python example where we serialize and deserialise the BT with json. ref
Line 189 in 341b7a7
I run the static type checker,
Done. |
|
@Sollimann both python package and rust package will be called |
Added python bindings for
bonsai-bt- also calledbonsai-bt. The new folder comes with itstests/andexamples/, where theexamples/folder should mimic the existing ones in rust. Since the original rust code is still small in size, writing the python bindings was not the hard part, in fact the bindings themselves are a small part of this PR. The hard part was all the devops work surrounding it. Namely:Opening up the PR early for reviews. What is left now:
bonsai-btproject.bonsai-btrelease, verify by being able to dopip install bonsai-btand running a test python script.@Sollimann I am stopping at this point for reviews. I will go ahead with setting up the above python publishing workflow if you are happy with what we currently have. For referece, once the above is done, all future releases workflow will look like this:
bonsai-py/Cargo.tomlversion. Merge to main. For this example lets assume the version is 0.13.0git tag py-test-0.13.0 && git push origin py-test-0.13.0→ TestPyPI dry-run.pip install -i https://test.pypi.org/simple/ bonsai-bt==0.13.0in a clean venv.git tag py-v0.13.0 && git push origin py-v0.13.0→ real PyPI release.pip install --upgrade bonsai-btand get latest version.Disclaimer: Used claude code to generate documentation, verify 100% test coverage, and generate plans for devops automation.