python: migration of oslquery python bindings away from OpenImageIO types - #2142
Open
lgritz wants to merge 1 commit into
Open
python: migration of oslquery python bindings away from OpenImageIO types#2142lgritz wants to merge 1 commit into
lgritz wants to merge 1 commit into
Conversation
…ypes OpenImageIO is moving from pybind11 to nanobind. We'd like to do the same with OSL as well. But one thing that will be a big inconvenience if either one switches is that because OSL's OSLQuery API uses OIIO's TypeDesc, that makes it very awkward to mix an OIIO that uses nanobind with an OSL that uses pybind11, or vice versa. I have verified that mixing and matching on main (before this PR) DOES NOT WORK. This PR provides an `OSLQuery.Parameter.type_name` property that holds the parameter's type as a string, rather than a TypeDesc. This will allow Python programs using OSLQuery to not need TypeDesc to have a compatible representation in the Python bindings of both packages, and thus allowing them to not have a problem with mixed pybind11/nanobind usage by the two packages, whichever versions they are using. This is the one and only spot in which OSL's OSLQuery python API *requires* use of anything provided by the OIIO python bindings. I have verified that this change allows peaceful coexistence between a new nanobind-based OIIO and the still-pybind11 OSL (or vice versa, presumably) -- as long your use of OSLQuery in python uses the new `Parameter.type_name` and not the old `Parameter.type`. We also no longer force an import of OpenImageIO when OSL's python bindings initialize. Again, the only time that is needed is if users actually use the old `type` property (rather than the new, preferred `type_name`), and presumably in that case, the TypeDesc had to come from somewhere that needed the OpenImageIO module loaded, so it seems reasonable to not need us to do it unconditionally. This ONLY affects the OSLQuery python bindings. The C++ bindings are a different can of worms entirely, since it makes extensive use of ustring in addition to TypeDesc. Disentangling them on the C++ side is going to be more delicate and probably will involve breaking changes. I think this is safe to backport as well, which will allow people using OSL 1.15 (as long as it's new enough to have the fix) to start using the new type_name and future-proof their scripts so that no changes are needed when the future nanobind based bindings come to either or both packages. Signed-off-by: Larry Gritz <lg@larrygritz.com>
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
1 participant
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.
OpenImageIO is moving from pybind11 to nanobind. We'd like to do the same with OSL as well. But one thing that will be a big inconvenience if either one switches is that because OSL's OSLQuery API uses OIIO's TypeDesc, that makes it very awkward to mix an OIIO that uses nanobind with an OSL that uses pybind11, or vice versa.
I have verified that mixing and matching on main (before this PR) DOES NOT WORK.
This PR provides an
OSLQuery.Parameter.type_nameproperty that holds the parameter's type as a string, rather than a TypeDesc. This will allow Python programs using OSLQuery to not need TypeDesc to have a compatible representation in the Python bindings of both packages, and thus allowing them to not have a problem with mixed pybind11/nanobind usage by the two packages, whichever versions they are using. This is the one and only spot in which OSL's OSLQuery python API requires use of anything provided by the OIIO python bindings.I have verified that this change allows peaceful coexistence between a new nanobind-based OIIO and the still-pybind11 OSL (or vice versa, presumably) -- as long your use of OSLQuery in python uses the new
Parameter.type_nameand not the oldParameter.type.We also no longer force an import of OpenImageIO when OSL's python bindings initialize. Again, the only time that is needed is if users actually use the old
typeproperty (rather than the new, preferredtype_name), and presumably in that case, the TypeDesc had to come from somewhere that needed the OpenImageIO module loaded, so it seems reasonable to not need us to do it unconditionally.This ONLY affects the OSLQuery python bindings. The C++ bindings are a different can of worms entirely, since it makes extensive use of ustring in addition to TypeDesc. Disentangling them on the C++ side is going to be more delicate and probably will involve breaking changes.
I think this is safe to backport as well, which will allow people using OSL 1.15 (as long as it's new enough to have the fix) to start using the new type_name and future-proof their scripts so that no changes are needed when the future nanobind based bindings come to either or both packages.