Improve types of childen, tracks, effects and markers parameters in function signatures #1448
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.
Summarize your change.
Continuation of the work I've been doing to improve the parameter types in our bindings.
In this PR, I'm changing the types of the
children
,tracks
,effects
andmarkers
parameters. A nice side effect is that we no more need the Python > C++ > Python > C++ > Python dance to convert the inputs from apy::object
andAnyVector
s.Previously, there was an hand-coded bridging to
AnyVector
while now it's using facilities in pybind11 to accomplish the same thing. Part of the reason that's possible, is thatpy::object
has been replaced in the interfaces with explicit types that pybind11 knows how to deal with.Here is an example of why it's important to expose the correct types in constructors (and functions/methods):
The previous behavior was something like this:
In this example there is two things to note. First, the error doesn't tell us which argument is of wrong type. Second, it doesn't tell us which specific types should be in the list/sequence. From the code, the list/sequence passed to the children parameter is not all subclasses of
SerializableObject
. It only accepts a list/sequence of typeComposable
and its sublcasses.Compare this to the new behavior:
Now we can at least see all the arguments and the list of all accepted signatures. The signature is clear about that fact that it accepts a list of
Composable
and its subclasses. Pybind11 doesn't tell us which argument is wrong, but that could be contributed to Pybind11 if we feel like it would improve the experience.There is one unanswered question though, at least for me, and it's what about the crashes mentioned in
otio_utils.cpp
? Andl also, what aboutEvery element has to be a SerializableObject::Retainer<>
? Was all this needed/casued by the fact that we were round tripping to Python?