python: nanobind tidying, auto-build, CI testing - #5346
Open
lgritz wants to merge 1 commit into
Open
Conversation
A collection of changes related to our switch from pybind11 to
nanobind. None of these change the binding code itself! Mostly
related to build and testing odds and ends.
* CI: Build and test both python back ends! Except for Windows, only
do nanobind there, since it's really slow to build the pybind11
bindings.
* auto-builder for nanobind if not found
- src/cmake/build_nanobind.cmake, mirroring build_pybind11.cmake
- With auto-build of nanobind, no need to use pip to install nanobind.
- Take care to share robin-map with OIIO core (both OIIO and nanobind
need robin-map).
* CMakeLists.txt : Change OIIO_PYTHON_BINDINGS_BACKEND initialization
to use set_cache() to allow env variables to easily control it.
* Testing :
- Fixes that make the tests run properly when only nanobind is built
(it previously worked for "both" and of course for "pybind11").
- Add tests to the nanobind set that were overlooked: filters,
docs-examples-python, and the hwy-enabled python-imagebufalgo variant.
- Add missing PYTHONPATH to python-imagebufalgo.hwy and openexr-copy
tests, which relied on the ambient shell already having it set
(true in CI via ci-startup.bash, not for a plain local ctest run).
- Semi-related cleanup: Don't have docs-examples-python depend on
docs-examples-cpp having already run to create its ref symlink;
create it ourselves if missing. Fixes failures when running
testing subset that includes docs-examples-python but not
docs-examples-cpp.
* pythonutils.cmake: comment out a now-redundant find_package call for
nanobind, since externalpackages.cmake already resolves it before this
macro runs.
* Fix discover_nanobind_cmake_dir(): it was a macro, so its return
could abort the entire including file (externalpackages.cmake)
rather than just itself, whenever a stale cached nanobind_DIR was
present -- silently skipping every dependency check after the Python
section. Convert to function.
* Robin-map:
- Raise robinmap minimum to 1.3, as needed by nanobind.
- Remove our bespoke FindRobinmap.cmake module in favor of using
modern robinmap's exported cmake configs (including cleanup of the
changed target names).
* Suppress nanobind reference leak warnings for python 3.9, allow for
3.10+. We were only getting these for 3.9, not newer Python, so
discussion with Wenzel concluded that it's probably just an old
python cleanup problem and not worth tracking down unless we see it
surface with a newer Python release.
Assisted-by: Claude Code / Sonnet 5
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Collaborator
Author
|
@soswow I would love your review of this, since it builds directly on your work. |
Contributor
by the end of my today 👍 |
Collaborator
Author
|
This is in a sense not directly related to the present PR, but people using both OIIO and OSL may be interested in AcademySoftwareFoundation/OpenShadingLanguage#2142 that is relevant to how we're going to migrate the projects safely to nanobind, since mixing and matching an OIIO and OSL where one uses pybind11 and the other uses nanobind will fail if you use any OSL API calls that involve types imported from OIIO (luckily, there is only one such call!). |
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.
A collection of changes related to our switch from pybind11 to nanobind. None of these change the binding code itself! Mostly related to build and testing odds and ends.
Sorry for the large number of things mixed together; each change individually either does nothing or doesn't work on its own, they are a collection of things that are all needed to fully enable working nanobind across all our CI tests.
CI: Build and test both python back ends! Except for Windows, only do nanobind there, since it's really slow to build the pybind11 bindings.
auto-builder for nanobind if not found
CMakeLists.txt : Change OIIO_PYTHON_BINDINGS_BACKEND initialization to use set_cache() to allow env variables to easily control it.
Testing :
pythonutils.cmake: comment out a now-redundant find_package call for nanobind, since externalpackages.cmake already resolves it before this macro runs.
Fix discover_nanobind_cmake_dir(): it was a macro, so its return could abort the entire including file (externalpackages.cmake) rather than just itself, whenever a stale cached nanobind_DIR was present -- silently skipping every dependency check after the Python section. Convert to function.
Robin-map:
Suppress nanobind reference leak warnings for python 3.9, allow for 3.10+. We were only getting these for 3.9, not newer Python, so discussion with Wenzel concluded that it's probably just an old python cleanup problem and not worth tracking down unless we see it surface with a newer Python release.
Assisted-by: Claude Code / Sonnet 5