Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert enumerations to Python enum.Enum subclasses #407

Closed
reinecke opened this issue Jan 9, 2019 · 3 comments
Closed

Convert enumerations to Python enum.Enum subclasses #407

reinecke opened this issue Jan 9, 2019 · 3 comments
Projects
Milestone

Comments

@reinecke
Copy link
Collaborator

reinecke commented Jan 9, 2019

We have some enumerated types spread throughout the codebase but they are treated as simple classes (see TrackKind, NeighborGapPolicy, MarkerColor to name a few).

Since these are currently simple classes effectively defining constants, the typing can become loose and the fact that enums are expected can get lost. One example is in the opentimelineio.schema.Track docstring:

class Track(opentimelineio.core.composition.Composition)
    Track(name=None, children=None, kind='Video', source_range=None, markers=None, effects=None, metadata=None)

The kind kwarg is actually opentimelineio.schema.TrackKind.Video.

In the case of NeighborGapPolicy the result is less readable because it maps to integers.

Python enum.Enum subclasses could be used to preserve and convey the typing. This may also add for nicer parity with the C++ implementation while keeping a pythonic api footprint and making possible values more discoverable to new API users.

Considerations:

  • For python 2.7 compatability, we'd need to introduce a dependency on enum34 (related, we should discuss allowing a dependency on six for other compatibility cases. See the HLS adapter and the six alternative).
  • This could be a breaking change if existing code in the wild is using the underlying primitives or have defined their own new values for them.
  • Do enums need to be versioned when new values are added or can we rely on the objects using the enumerations to include that in their schema versions (e.x. if we add a new TrackKind increment the Track schema version)?
@ssteinbach
Copy link
Collaborator

I think this is going to change in the C++ based version. @davidbaraff do you know how these enums from C++ are wrapped in python?

@ssteinbach ssteinbach added this to the Public Beta 11 milestone Apr 19, 2019
@ssteinbach ssteinbach added this to To Do in C++ API via automation Apr 19, 2019
@ssteinbach
Copy link
Collaborator

@reinecke does this still need to happen in light of the C++ branch?

@reinecke
Copy link
Collaborator Author

It looks like NeighborGapPolicy already transitioned to an enumeration via the bindings.

For TrackKind and MarkerColor, as time goes on I think we've clarified that these aren't enumerations, they're constants for commonly used strings. I think we're embracing the power in allowing people to define whatever MarkerColor or TrackKind they need - most times one of the constant values from those classes will work though.

C++ API automation moved this from To Do to Done Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
C++ API
  
Done
Development

No branches or pull requests

2 participants