-
Notifications
You must be signed in to change notification settings - Fork 57
adds nanobind scaffolding for future python bindinds #205
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
adds nanobind scaffolding for future python bindinds #205
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #205 +/- ##
=======================================
Coverage 72.27% 72.27%
=======================================
Files 10 10
Lines 2348 2348
Branches 311 311
=======================================
Hits 1697 1697
Misses 651 651 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| OUTPUT_NAME "rawtoaces" | ||
| #EXPORT_NAME "rawtoaces" | ||
| #SOVERSION ${RAWTOACES_MAJOR_VERSION}.${RAWTOACES_MINOR_VERSION}.${RAWTOACES_PATCH_VERSION} | ||
| #VERSION ${RAWTOACES_VERSION} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments. needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I think the binary should be versioned, but uncommenting that makes configure fail.
Also I don't know if this whole 'export' thing is needed, I use it to change the library name, as the 'rawtoaces' target name is already taken. This also makes the library install with the rest of targets, I don't know if the destination should be different. This is good enough for now. We'll need somebody more familiar with python tools deployment to finalise this, I suppose.
| ceres:x64-windows \ | ||
| nlohmann-json:x64-windows \ | ||
| openimageio:x64-windows | ||
| openimageio:x64-windows \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this builds and installs the OIIO python bindings by default. You may need some kind of modifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. Will check. We don't need OIIO on the python side that right now.
Signed-off-by: Anton Dukhovnikov <[email protected]>
Signed-off-by: Anton Dukhovnikov <[email protected]>
9815a19 to
c3b8e05
Compare
bbb6200
into
AcademySoftwareFoundation:main
This adds all necessary code to start working on python bindings.
rawtoaces_bindingsNote, I've only added minimal bindings necessary to convert an image. The stuff I've implemented:
I have made sure that the code builds on all platforms on CI, but I have only tested the python code on my machine (Mac).
Here is how I tested:
PYTHON_PATHand/orDYLD_LIBRARY_PATH, this may be platform-dependent,Next steps from here:
rta::util::ImageConverter, which do not use any OpenImageIO types as parameters or return values. The bindings in OIIO are currently implemented with pybind, but there is a plan to rewrite them with nanobind.rta::core::SpectralSolverandrta::core::MetadataSolver.OIIO::ImageBuf-based API by converting the ImageBufs to/from numpy arrays.OIIO::ImageBuf-based API by exposing the OIIO types on the python side.