OpenTimelineIO API observations

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:

1. Someone looking at modifying these method will have to search where they are defined.
2. Import time cost.
3. 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.

Some notes on individual sections:

py::object

For metadata and children arguments, it may take some pybind research to find a suitable replacement. I think those are using py::object to properly implement duck typing behavior. If I were using type hints in python metadata would be typing.Mapping[str, Any] and children would be typing.Sequence[otio.schema.Item]. Note that this would allow doing things like passing a dictionary where the keys are OTIO clips as the children, which is totally valid. If this were a language like swift, we'd use protocols to communicate this type info.

Enums or not Enums

#407 has some discussion around this.

There is an important distinction in the example cases, color and track kind "enums" are actually more like a collection of string constants. It's valid OTIO to have any arbitrary string for your marker color or track kind - I think this is a good thing. The faux-enums offer a set of commonly-used values that a lot of people interpret in specific ways but allows for expansion.

We should double-check others are aligned on this view, however. The CMX adapter seems to take a different opinion.

I'm not sure how much context I have on the other issues so I'll leave those for others to discuss.

If I were using type hints in python metadata would be typing.Mapping[str, Any]
@reinecke Technically it wouldn't be any right? From what I understand, the accepted list of types for the metadata values is https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/3e4b4591409a4afce92ca7b2bc9cb94c7f756426/src/py-opentimelineio/opentimelineio/core/_core_utils.py#L22. Am I right?

Edit: Thanks for taking time to read this and comment!

Technically it wouldn't be any right?

@JeanChristopheMorinPerso Yes, that's correct. It'd be something more like Union[str, int, float, SerializableObject, RationalTime, TimeRange, etc.] with the types you linked. I realized I was thinking of our C++ Any mechanism under the hood that implements this - but even that isn't technically the exact right constraints.