Added docstrings to python bindings for interval algebra methods in TimeRange

[reinecke]

Also re-arranged the overloaded definitions so the TimeRange/RationalTime variants would have consistent ordering.

Here is a summary of what the docstrings in python look like now:

 |  before(...)
 |      before(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. before(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.RationalTime, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The end of :param:`this` strictly precedes :param:`other` by a value >= :param:`epsilon_s`.
 |                             other
 |                               ↓
 |                   [ this ]    *
 |
 |
 |      2. before(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The end of :param:`this` strictly equals the start of :param:`other` and
 |      the start of :param:`this` strictly equals the end of :param:`other`.
 |                   [this][other]
 |      The converse would be ``other.meets(this)``
 |
 |  begins(...)
 |      begins(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. begins(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.RationalTime, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The start of :param:`this` strictly equals :param:`other`.
 |                 other
 |                   ↓
 |                   *
 |                   [ this ]
 |
 |
 |      2. begins(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The start of :param:`this` strictly equals the start of :param:`other`.
 |      The end of :param:`this` strictly precedes the end of :param:`other` by a value >= :param:`epsilon_s`.
 |                   [ this ]
 |                   [    other    ]
 |      The converse would be ``other.begins(this)``
 |
 |  contains(...)
 |      contains(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. contains(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.RationalTime) -> bool
 |
 |
 |      The start of :para:`this` precedes :param:`other`.
 |      :param:`other` precedes the end of this.
 |                          other
 |                            ↓
 |                            *
 |                    [      this      ]
 |
 |
 |      2. contains(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The start of :param:`this` precedes start of :param:`other`.
 |      The end of :param:`this` antecedes end of :param:`other`.
 |                        [ other ]
 |                   [      this      ]
 |      The converse would be ``other.contains(this)``
 |
 |  finishes(...)
 |      finishes(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. finishes(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.RationalTime, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The end of :param:`this` strictly equals :param:`other`.
 |                        other
 |                          ↓
 |                          *
 |                   [ this ]
 |
 |
 |      2. finishes(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The start of :param:`this` strictly antecedes the start of :param:`other` by a value >= :param:`epsilon_s`.
 |      The end of :param:`this` strictly equals the end of :param:`other`.
 |                           [ this ]
 |                   [     other    ]
 |      The converse would be ``other.finishes(this)``
 |
 |  intersects(...)
 |      intersects(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The start of :param:`this` precedes or equals the end of :param:`other` by a value >= :param:`epsilon_s`.
 |      The end of :param:`this` antecedes or equals the start of :param:`other` by a value >= :param:`epsilon_s`.
 |              [    this    ]           OR      [    other    ]
 |                   [     other    ]                    [     this    ]
 |      The converse would be ``other.finishes(this)``
 |
 |  meets(...)
 |      meets(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The end of :param:`this` strictly equals the start of :param:`other` and
 |      the start of :param:`this` strictly equals the end of :param:`other`.
 |                   [this][other]
 |      The converse would be ``other.meets(this)``
 |
 |  overlaps(...)
 |      overlaps(*args, **kwargs)
 |      Overloaded function.
 |
 |      1. overlaps(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.RationalTime) -> bool
 |
 |
 |      :param:`this` contains :param:`other`.
 |                        other
 |                         ↓
 |                         *
 |                   [    this    ]
 |
 |
 |      2. overlaps(self: opentimelineio._opentime.TimeRange, other: opentimelineio._opentime.TimeRange, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      The start of :param:`this` strictly precedes end of :param:`other` by a value >= :param:`epsilon_s`.
 |      The end of :param:`this` strictly antecedes start of :param:`other` by a value >= :param:`epsilon_s`.
 |                   [ this ]
 |                       [ other ]
 |      The converse would be ``other.overlaps(this)``

[ssteinbach]

Is the :param: syntax for something specific?

[ssteinbach]

Spotted a typo, otherwise looks good. Does it build all the way through to RtD?

[reinecke]

The docstrings did have a rendering issue in sphinx. As a note, the majority of the C++ codebase isn't getting picked up by sphinx, I've filed #790 for that.

To check these docs, I commented out this line:
https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/30530a20b476e222c27b1d69073137f2d9502cc9/docs/conf.py#L304

Then added the following to the auto-generated files in api/modules/opentimelineio.rst:

.. autoclass:: opentimelineio.opentime.TimeRange
   :members:
   :undoc-members:
   :show-inheritance:
   :private-members:

Here's an example of the rendered output:

[ssteinbach]

Docs look good. Do you know if there is a way of getting the function signature to python to not appear as the *args, **kwargs form? @meshula maybe you know if there is a trick in Pybind11 to get that to advertise correctly?

[meshula]

The fact that it renders

.def("contains", (bool (TimeRange::*)(RationalTime) const) &TimeRange::contains, "other"_a)

via kwargs, rather than something friendly is a surprise to me. Maybe we should open an issue on the Pybind11 repo?

[reinecke]

Because I'm a bit crazy, I've worked through another possible solution. The end result looks like this:

 |  before(...)
 |      before(self: opentimelineio._opentime.TimeRange, other: object, epsilon_s: float = 2.6041666666666666e-06) -> bool
 |
 |
 |      If other is `otio.opentime.RationalTime`:
 |
 |      The end of `this` strictly precedes `other` by a value >= `epsilon_s`.
 |      ::
 |                             other
 |                               ↓
 |                   [ this ]    *
 |
 |      If other is `otio.opentime.TimeRange`:
 |
 |      The end of `this` strictly equals the start of `other` and
 |      the start of `this` strictly equals the end of `other`.
 |      ::
 |                   [this][other]
 |      The converse would be ``other.meets(this)``
 |

Or in the mkdocs render:

To achieve this, I implemented the before binding using a wrapper function instead of overloading:

        .def("before", [](TimeRange self, py::object other, double epsilon) {
                try {
                    auto tr = other.cast<TimeRange>();
                    return self.before(tr);
                } catch (py::cast_error e) {
                    try {
                        auto rt = other.cast<RationalTime>();
                        return self.before(rt);
                    }
                    catch (py::cast_error e) {
                        throw py::type_error("before() argument must be TimeRange or RationalTime");
                    }
                }
            }, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s, R"docstring(
If other is `otio.opentime.RationalTime`:

The end of `this` strictly precedes `other` by a value >= `epsilon_s`.
::
                       other
                         ↓
             [ this ]    *

If other is `otio.opentime.TimeRange`:

The end of `this` strictly equals the start of `other` and
the start of `this` strictly equals the end of `other`.
::
             [this][other]
The converse would be ``other.meets(this)``
)docstring")

If we decide this is the option we want to go with, I'd love some guidance on the idiomatic C++ way to make a reusable form of this since it's used over and over for many methods. It feels like there might be a solution with either templates or method pointers?

[meshula]

With the existing implementation, the legality of RationalTime and TimeInterval is explicit, and can be discovered by introspecting an object.

A downside of this casting method is that the legality of RationalTime is hidden from the runtime, and the invocation of the RationalTime variant is a side effect of exception handling. That would be a C++ anti-pattern. More idiomatically, you could ask the other template if a particular cast is legal, and then branch accordingly without invoking exceptions. That said, even casting checks are expensive.

Ultimately I think hiding the legality of RationalTime from the API is probably too much of a design cost.

Jamming on your idea of a binding, I wonder if there is some other wrapping pattern that preserves the disclosure of legal types in the API, but avoids the ugliness of the kwargs in the autogenerated docs?

[ssteinbach]

@reinecke @meshula My feeling on this is to land this fix and file an issue to improve the bindings at a later time to remove the *args, **kwargs stuff, this is already a nice improvement in documentation. Sound good to you folks?

[meshula]

lgtm

[ssteinbach]

@reinecke @meshula I made an issue to track the discussion around getting away from the *args and **kwargs: #870