Unify pybind11 and nanobind into single-source bindings#5254
Open
soswow wants to merge 2 commits into
Open
Conversation
eb2308d to
4bf64a7
Compare
Added support for nanobind as a backend for Python bindings alongside pybind11. Updated CMake configuration to conditionally include Python modules based on the selected backend. Refactored existing Python binding declarations to utilize a unified interface for both backends. Introduced a new MIGRATION_STATUS.md file to document the migration process and current status of nanobind integration. Enhanced installation paths for nanobind modules to ensure proper isolation and organization within the build structure. This change aims to improve flexibility in Python bindings and facilitate future development with nanobind. Assisted-by: Cursor / auto Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
Refactored Python binding macros in the py_backend.h and related files to use a more consistent syntax with dot notation. This change enhances readability and maintains uniformity across the codebase. Adjusted the ImageSpec, ParamValue, ROI, and TypeDesc classes to utilize the new macro definitions. Minor formatting improvements were also made for better code clarity. Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
5cf873d to
a78db1c
Compare
Contributor
|
Thanks for doing this. I prefer this version to the previous one. |
Contributor
Author
100% I like it much more. I am still learning and I had wrong impression that they are very different |
Contributor
Author
|
Btw, I've tried converting rest of the modules to nanobind in this style to see how this would go and it is surprisingly small diff: |
Contributor
Author
|
Here is a draft PR for adding more tests to improve chances of catching problems and minimize risk of regression in production pybind11 - |
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.
Exploring an idea Anton had about unifiying two python backends into single source since they are so similar.
Summary
This PR unifies the early nanobind migration with the existing pybind11 Python bindings by compiling shared sources from
src/python/for both backends, instead of maintaining parallel copies undersrc/python-nanobind/.Migrated modules so far:
ROI,TypeDesc,ImageSpec,ParamValue/ParamValueList, plus the coreOpenImageIOmodule helpers inpy_oiio.cpp. Backend differences are isolated inpy_backend.h(type aliases, binding API macros like.OIIO_PY_RW(...), and smalloiio_py::helpers). Shared Python↔C++ conversion logic lives inpy_oiio.hwith#ifonly where the backends genuinely differ.CMake is updated so that when
-DOIIO_PYTHON_BINDINGS_BACKEND=nanobindis selected, the module is built from the unified sources and installed as the mainOpenImageIOpackage in the expected location. Withboth, pybind11 remains the primary module and nanobind builds asPyOpenImageIONanobindfor side-by-side testing.Duplicate nanobind-only
.cppfiles are removed (~1.8k lines deleted net). Testsuite coverage is extended with explicit roundtrip checks for shared helper paths (attributes, buffers, tuples).Pros
py_roi.cpp,py_imagespec.cpp, etc. in sync by hand.src/python-nanobind/while centralizing shared logic in one place.MIGRATION_STATUS.mddocuments what is fully migrated, partially migrated, and still pybind-only.main, choosing nanobind does not install into the same package layout as pybind11; this PR fixes that for the migrated surface.OIIO_PYTHON_BINDINGS_BACKEND=bothbuilds both modules so existingtestsuite/python-*and*.nanobindvariants can be run against the same C++ sources..OIIO_PY_RW(...)) keep clang-format happy withoutclang-format offregions.Cons
py_backend.h, macros, and shared helpers; slightly more indirection than plain.def_readwrite(...).ImageBuf,ImageInput/ImageOutput,ImageBufAlgo, texture system, etc.) is still pybind-only; this PR is a foundation, not full nanobind coverage.MIGRATION_STATUS.mdare still missing or intentionally different vs pybind11..nanobindvariants for touched code.#if defined(OIIO_PY_BACKEND_NANOBIND)remains in a few places (e.g.ParamValueconstructors, buffer dispatch); easy to get wrong if extended carelessly..OIIO_PY_RW("x", ...)reads less directly than.def_readwrite("x", ...)until you know the macro map.