-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add nanobind Python bindings #439
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Max Lantas <[email protected]>
This is awesome, thanks!! I'll look through it in greater detail shortly. |
I edited the CI matrix to build the While I was working on initial PR, I also tried get the unit tests running for both versions of the bindings, but couldn't figure out how to structure the CMake files to import the generated Python module without an error. |
Signed-off-by: Max Lantas <[email protected]>
I'm unclear on the big picture strategy with regard to pybind11 and nanobind. Are you proposing that we build both sets? Or that we switch from pybind11 to nanobind completely? What's the compatibility story between the two? If some other project that uses pybind11 and expects to use the Imath data types from Python, does Imath switching to nanobind interfere with that? Do we have an enumeration of the reasons why a project would not be able to switch from pybind11 to nanobind? |
Hi @lgritz, The goal is to replace I don't believe there's a compatibility problem. The choice of Additionally, all of the |
But what about another C++ library that has a pybind11-mediated Python API, whose public APIs use the Imath classes? I feel like at some point somebody told me that this was not going to work if one was nanobind and the other was pybind11. But I don't know enough to say why that is a problem. |
Oh, I see what you're saying. In that case, no,
If the reason for introducing |
To answer your earlier question, the only reason an external project can't switch to |
A major issue is that nanobind ONLY allows python bindings of a C++ library. But pybind11 could do some other tricks, including letting you embed a python interpreter into an application. That's another one I'm very concerned about. Anyway, there's no reason why nanobind isn't suitable for Imath in isolation. But I worry that it's unsuitable for other things that need to use Imath classes in their public APIs, or that need to embed python as an extension language for applications. It feel like it's possible that what we'd need to do is try to get the whole local ecosystem (ASWF projects and some siblings) to all switch from pybind11 to nanobind, or else always need to deal with incompatibilities cropping up. |
There are at least a few instances of this in the ASWF repos: https://github.com/search?q=org%3AAcademySoftwareFoundation+language%3AC%2B%2B+pybind11%2Fembed&type=code I suppose those files could be rewritten to use the CPython embedding API directly since that's basically what
Yes, that seems like the only way a I wouldn't be surprised if at least one of the projects used other I don't see a path forward for this pull request. |
Catching up on this now. This is the kind of analysis I was hoping to see, so it's a helpful contribution even if it goes no further. The reason we're considering this in the first place is the desire to drop the current dependence on Boost.Python. Any new release of Imath without Boost.Python is going to require a major rewrite on the part of projects that uses the current Boost.Python PyImath C++ classes. ABI compatibility was never an objective. The thought was, if you're going to rewrite, would there be advantages to rewriting for nanobind instead of pybind11? Alembic is the only such open source library I'm aware of, but there may be others. The other pybind11-based ASWF projects don't currently use PyImath's C++ API that I'm aware of, although maybe they would if there weren't compatibility problems. @lamiller0 contributed the original pybind11 Imath WIP so presumably there was some thought about the implications for Alembic. I'm still unclear on exactly what this means in relation to Imath and to projects like Alembic that use the C++ Imath API, regarding pybind11 features missing in nanobind:
The next bit of evaluation to consider is exactly what the port of Alembic to a pybind11-based Imath would look like, and what wrinkles a nanobind alternative would introduce there. |
I think a minimal external C++ binding library that provides 1 method that depends on PyImath bindings to take a list of V3d from python, use those to calculate the bounds and then return that to python should be good enough to demonstrate external compatibility for most other libraries. Alembic is mostly reliant on PyImath for the usual data types, V3f, V3d, M33d, M44d, etc. We had a first pass at a PR for supporting pybind11 but it never made its way into release because we were unsure of how to ingest PyImath in a familiar way. It doesn't need to stay pybind11, we would be willing to switch over to a different binding as long as it was compatible with a future PyImath. |
I'll try to prepare this in the near future. If Alembic is the only major project to bind against the Imath API in this way, then that certainly changes the scope of this change. |
Sorry for the delay. I implemented a small example external library in Both the
Couldn't figure out the proper way to reference Imath from an external CMake project, but if you install this branch's version of Imath to
As expected, mixing and matching As long as Imath and Alembic can agree on which binding library to use, I don't see anything blocking a migration away from |
Thank you for the analysis, it helps clarify the situation. I'm inclined to say that Imath should continue migrating to pybind11 and drop consideration of nanobind, but it's worth a larger discussion within the ASWF to confirm that's the consensus direction. And we'll be sure to coordinate with Alembic. Since Imath is a rather static library, not expected to evolve significant new functionality, it's not out of the question to continue to support multiple binding libraries. |
Chiming in as an outsider but someone involved with both binding projects. I strongly recommend against starting new projects on pybind11 and migrating existing projects to nanobind if possible. Pybind11 has severe and unfixable problems. It will only get worse in the future with the significant changes happening in the Python ecosystem. |
"as someone involved with both binding projects" |
@wjakob Unfortunately, to the best of my knowledge, nanobind's minimum python version is 3.8. A lot of our projects have, ugh, only recently been able to drop python 2.7, with 3.7 being the new minimum and quite frequently a constraint. So it might take a bit before we can solidly consider Python 3.9 our new minimum and thus fully unlock the possibility of switching to nanobind. On a slightly different point, is there really no way to make nanobind and pybind11 able to converse? The whole migration could go faster it if could be done incrementally, project by project. The need to have lots of projects switch either simultaneously or not at all definitely holds things back. I'm thinking of cases where there are projects A and B, and B needs to use public types of A. They both have to be nanobind or both pybind11, but not mixed, correct? |
Python 3.7 is difficult to support since it doesn't have vector calls, which are a key architectural feature driving nanobind (those were introduced in 3.8. While nanobind supports 3.8, that version is probably not a good target since it is EOL since Oct 2024). There has been some preliminary discussion abou basic interop. I can't yet say how that develop, but such a feature might alleviate the all-or-nothing requirement of porting a set of interconnected packages. You would really just need this though if you simultaneously want to use pybind11+nanobind extensions containing bindings of functions that take instances of the same C++ class (e.g. |
Closes #433
This PR adds a new target,
NanobindImath
for thenanobind
version of thepybind11
bindings. To build thenanobind
bindings, the-DNANOBIND
CMake option must be'ON'
.pybind11
bindings are still available.As expected,
nanobind
is almost entirely compatible with the previouspybind11
bindings with only a few differences:def_readwrite
member function is renamed todef_rw
module
type is renamed tomodule_
. Probably so it doesn't collide with the C++20 modules keywords.nanobind_build_library(nanobind-static)
is necessary to build a static library for thelibNanobindImath...
shared library to link with.I couldn't test compilation time/runtime performance very reliably, but the resulting binaries are significantly smaller.
Less than half the size for the python module:
And less than 1/10 of the size for the shared library.
Both the
nanobind
andpybind11
bindings are fairly small compared to the Boost bindings, so these size ratios may not scale linearly, but I think it's promising.