JeanChristopheMorinPerso/OpenTimelineIO API observations.md

## OpenTimelineIO API observations.md

      
    Raw
  

              OpenTimelineIO API observations.md
            
          
    Python bindings

py::object as parameter type in class signatures

There is a pattern in the Python bindings (both opentime and opentimelineio) where parameters of type string, list and
dict and bool are typed as py::object.
The values are converted in the constructors using string_or_none_converter, py_to_vector and py_to_any_dictionary
utility functions.
One such example is the Track class:
py::class_<Track, Composition, managing_ptr<Track>>(m, "Track", py::dynamic_attr())
    .def(py::init([](py::object name,
                     py::object children,
                     optional<TimeRange> const& source_range,
                     std::string const& kind,
                     py::object metadata) {
                         auto composable_children = py_to_vector<Composable*>(children);
                         Track* t = new Track(
                             string_or_none_converter(name),
                             source_range,
                             kind,
                             py_to_any_dictionary(metadata)
                         );
                         if (!composable_children.empty())
                             t->set_children(composable_children, ErrorStatusHandler());
                         return t;
                  }),
         py::arg_v("name"_a = std::string()),
         "children"_a = py::none(),
         "source_range"_a = nullopt,
         "kind"_a = std::string(Track::Kind::video),
         py::arg_v("metadata"_a = py::none()))
The main problem with this practice is that the signature becomes:
 |  __init__(...)
 |      __init__(self: opentimelineio._otio.Track, name: object = '', children: object = None, source_range: Optional[opentimelineio._opentime.TimeRange] = None, kind: str = 'Video', metadata: object = None) -> None

It is impossible to know what type of objects we can pass to the name, children and metadata parameters
from Python without actually trying it. For example:
>>> opentimelineio.schema.Track(name={}, children=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jcmorin/jcmenv/aswf/OpenTimelineIO/build/lib.linux-x86_64-3.10/opentimelineio/core/_core_utils.py", line 152, in _value_to_so_vector
    raise TypeError(
TypeError: Expected list/sequence of SerializableObjects; found type '<class 'int'>'
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 type Composable and its sublcasses.
It is also good to note in this example the Python > C++ > Python > C++ roundtrip to get the type error.
Another example, but this time in opentime:
.def("to_timecode", [](RationalTime rt, double rate, py::object drop_frame) {
        return rt.to_timecode(
                rate,
                df_enum_converter(drop_frame),
                ErrorStatusConverter()
        );
}, "rate"_a, "drop_frame"_a)
where df_enum_converter is:
IsDropFrameRate df_enum_converter(py::object& df) {
    if (df.is(py::none())) {
        return IsDropFrameRate::InferFromRate;
    } else if (py::cast<bool>(py::bool_(df))) {
        return IsDropFrameRate::ForceYes;
    } else {
        return IsDropFrameRate::ForceNo;
    }
}
In this case, it would be better to use optional<bool> and modifying df_enum_converter to take an optional bool would do the trick.
name parameter type differs between different classes

Some classes accept a name parameter of type py::string while others accept py::object.
Reference commit: https://github.com/PixarAnimationStudios/OpenTimelineIO/pull/540/commits/49f0b133847ed08f0958ee2d6cec7e1dd04dfcbf
Enums or not Enums?

Some enum-like classes are pure classes while others are real enums.
For example, opentimelineio.schema.Marker.Color and opentimelineio.schema.Track.Kind are standard classes while
opentimelineio.schema.Track.NeighborGapPolicy and opentimelineio.schema.ImageReference.MissingFramePolicy are real enums.
AcademySoftwareFoundation/OpenTimelineIO#407
__str__ and __repr__

Classes in opentimelineio._otio have their __str__ and __repr__ methods implemented in Python while their
class is implmented in C++.
This creates some problems:

Someone looking at modifying these method will have to search where they are defined.
Import time cost.
Method implementation is sperarated from the class implementation.

I think these methods could be defined in C++ without too much troubles. This is already done in opentime.
Opentime

opentimelineio.opentime exports 4 functions that are unnecessarily implemented in Python.

to_timecode
to_frames
to_seconds
to_time_string

I understand from these that they are there for backward compatibility reasons since they "wrap" methods
of the opentime.RationalTime` class.