AcademySoftwareFoundation / AcademySoftwareFoundation/OpenTimelineIO

Python API: should double type/[up|down]grade function registration raise an exception

Open
#1,391 0 comments 0 reactions 0 assignees View on GitHub
needs discussion
Dominant language
C++
Stars
2k
Forks
351
Avg merge
1d 12h
Merged PRs (30d)
1

Description

Following discussion from: #1387

The python binding for the registration system for types and up/downgrade functions currently returns a bool:

- https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/424967ae04701462ae13aa59897860f0f08f39a3/src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp#L39
- https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/424967ae04701462ae13aa59897860f0f08f39a3/src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp#L54

However, the bindings _have_ a system for capturing these results and raising exceptions. I briefly experimented with this during the development of #1387, but it seems like the python unit tests assume that its ok to double register the same type without it triggering an exception.

For unit testing this probably means additional functions that allow un-registering types from the `TypeRegistry`.

I think the action items are:
- [ ] discuss at a TSC meeting whether this would be desired behavior,
- [ ] ...and if so what exception should be raised

Contributor guide

Open the contributing guide

Research direction

Start with the registration bindings in src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp at the referenced lines, then review the Python unit tests that double-register types. The TSC must first decide whether duplicate registration should raise and which exception to use; done should include the agreed behavior and any needed TypeRegistry unregister support with corresponding tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
api
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.