AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.47k stars 293 forks source link

Rearrange PyPI projects, break out adapters #1386

Closed ssteinbach closed 5 months ago

ssteinbach commented 2 years ago

Overview

The goal of this issue is to refactor the PyPI projects for OpenTimelineIO into two pypi projects from the current one:

Outstanding tasks:

Versioning strategy

Why the OpenTimelineIO Does Not Include The Plugins ("OpenTimelineIO-core vs OpenTimelineIO-

Currently, if you pip install opentimelineio you get the contrib adapters + the core library. This suggestion switches that around such that pip install opentimelineio is only the core without the contrib adapters. We decided to make this shift for several reasons:

Question

mikemahony commented 2 years ago

Are we imagining all the packages (OpenTimelineIO umbrella, OTIO-core, and other adapters) would live in the same, existing repo? Or do we want them each split out into their own git repositories?

timlehr commented 2 years ago

I'd be in favour of splitting them into their own repos.

jminor commented 2 years ago

The adapters should definitely go into separate repos (one per adapter) so that experts in each one can be given ownership over them.

As for the core and the umbrella, I don't know. Is there a tradeoff worth considering? I can sort of imagine what a separate umbrella repo would look like, but I'm unclear on the details. What would it look like to have them together in one repo?

apetrynet commented 2 years ago

Is checkbox number two in the list the GitHub project itself or the project on pypi? I'm assuming the latter(?)

So when pip install OpenTimelineIO you get core and default adapters?

Would it make sense to follow the pip install OpenTimelineIO[core] style? Then we could do:

I'm not sure if this is possible though, as I don't have enough experience with setting such a structure up. The contrib key might be tricky to update without releasing new main packages?

And I agree about splitting adapters to separate repos

JeanChristopheMorinPerso commented 2 years ago

I would see the "core" as a separate package (but still in the opentimelineio repo), opentimelineio-core. Installing opentimelineio should install the core I think. From there, we have two options:

  1. Adapters are installed with pip isntall opentimelineio[adapters]. From what I remember we kind of already ruled this option out because the user experience isn't great.
  2. Adapters are installed with opentimelineio (with the core too). This gives a nice experience to users.

As for the core and the umbrella, I don't know. Is there a tradeoff worth considering? I can sort of imagine what a separate umbrella repo would look like, but I'm unclear on the details.

If the core and the meta package (the umbrella) live in different repos, I think it could make it confusing to users. Our main repo would be named OpenTimelineIO like it is right now. But the meta package repo... How would it be named? Also, wouldn't it be awkward to have a repo that doesn't match the package name? Though that would be the case for the core package, but I don't see that much as a problem for core.

One thing that we might want to consider is versioning and version constraints and how we will want to bundle things. Will the meta package define its dependencies with lower bound versions or without constraints at all? What happens when new features are added to OTIO (or breaking changes)? Should adapters be compatible with multiple versions of OTIO? In that same line of thought, should they use lower bound constraint (>=0.14)) on OTIO version or both lower and upper bounds (>=0.14, <=1.17)? (specifying a maximum would probably cause problem down the line).

jminor commented 2 years ago

@mikemahony and I made this empty repo for experimentation. We can sketch this out and see how it fits before deciding on the final layout: https://github.com/OpenTimelineIO/otio-umbrella

I agree that having the repos and the pypi packages with clear names is important. The version history of a separate umbrella repo seems appealing too, but not essential.

ssteinbach commented 2 years ago

Note that there are a few explicit references to the contrib package in the code:

https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/5e27e78177095b709a2864ed6dced46bd221ac53/src/py-opentimelineio/opentimelineio/plugins/manifest.py#L327

Search for "contrib" in manifest.py. I see some in the autogen documentation as well, might be some others:

https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/5e27e78177095b709a2864ed6dced46bd221ac53/src/py-opentimelineio/opentimelineio/console/autogen_plugin_documentation.py#L348

I added a todo list item to remove these references.

mikemahony commented 2 years ago

Putting these here for something to look at:

Option A - Core/Umbrella Separate Repositories
----------------------------------------------

 Owner            | Repository          | Package
 -----------------+---------------------+-----------------
  ASWF            |  OpenTimelineIO     |  otio-core
  OpenTimelineIO  |  otio-umbrella      |  opentimelineio
  OpenTimelineIO  |  otio-aaf-adapter   |  otio-aaf-adapter
  gilou           |  otio-drp-adapter   |  otio-drp-adapter
  apetrynet       |  otio-mlt-adapter   |  otio-mlt-adapter
  OpenTimelineIO  |  otio-xges-adapter  |  otio-xges-adapter   <-- owner ok?
  OpenTimelineIO  |  otio-ale-adapter   |  otio-ale-adapter   <-- owner ok?
  OpenTimelineIO  |  otio-rv-adapter    |  otio-rv-adapter   <-- owner ok?
  ...             |  ...                |  ...
Option B - Core/Umbrella Combined Repository
--------------------------------------------

 Owner            | Repository          | Packages
 -----------------+---------------------+-----------------
  ASWF            |  OpenTimelineIO     |  otio-core, opentimelineio (umbrella)
  OpenTimelineIO  |  otio-aaf-adapter   |  otio-aaf-adapter
  ...             |  ...                |  ...
reinecke commented 2 years ago

After discussing at the 11/10/2022 TSC meeting, the consensus is to try Option B and see how it works out.

mikemahony commented 2 years ago

@apetrynet, your name came up the other day - would you be interested and available to create repositories here for the individual contrib adapters? Perhaps even just placeholders/stubs with just enough to be able to be deployed to https://test.pypi.org?

ssteinbach commented 2 years ago

I added another todo list item based on the TSC discussion that a pinning strategy should be documented for the core, umbrella, and adapter repositories.

apetrynet commented 2 years ago

@mikemahony, Yes, I can look at this.

mikemahony commented 2 years ago

@markreidvfx, do you feel like https://github.com/OpenTimelineIO/otio-aaf-adapter is ready enough to be deployed to test.pypi.org?

markreidvfx commented 2 years ago

@mikemahony yes, I think it is in a good enough state to start testing that. I haven't verified that any of the pypi deployment scripts work, they will probably need tweaking.

I've been tracking that here and additional tasks here https://github.com/OpenTimelineIO/otio-aaf-adapter/issues/1

markreidvfx commented 2 years ago

@mikemahony are the any docs or examples you know of on how your suppose to use test.pypi.org vs the regular pypi? I've typically just used the dev tag on the regular pypi until I get the scripts right, then clean up my tracks :p

JeanChristopheMorinPerso commented 2 years ago

@markreidvfx you can add -r testpypi to the twine command, see https://twine.readthedocs.io/en/stable/.

markreidvfx commented 2 years ago

thanks @JeanChristopheMorinPerso. I think I get it now. I created the initial pypi project https://test.pypi.org/project/otio-aaf-adapter I can add others as owners if they slack me their pypi username.

apetrynet commented 2 years ago

@apetrynet, your name came up the other day - would you be interested and available to create repositories here for the individual contrib adapters? Perhaps even just placeholders/stubs with just enough to be able to be deployed to https://test.pypi.org?

Hi! Should I set up repos for the contrib adapters or would it perhaps make more sense to start with a couple of the intended "batteries" like cmx3600 and fcp_xml? I ask partly because these are adapters the otio project maintains(?) and also because some of the contrib ones are in the works for extraction by their authors.

jminor commented 2 years ago

You can start with the non-contrib ones, so we can demonstrate that this all works before deciding about each contrib adatper.

apetrynet commented 1 year ago

I've created a branch called "extract_adapters" where we can safely remove adapters and use for testing the extracted plugins. I'm also in the process of submitting a PR where I've removed (what I believe are) all cmx_3600 related files and tests. A cmx3600 plugin repo is on the verge of getting ready for transfer to the OpenTimelineIO org

jminor commented 1 year ago

Excellent! Let's get @markreidvfx 's AAF adapter PR https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1348 and https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1487 landed into that branch.

apetrynet commented 1 year ago

The cmx_3600 adapter now has its own repo

apetrynet commented 1 year ago

I'll get started on the fcp_adapter.

apetrynet commented 1 year ago

FCP adapter repo transferred to OpenTimelineIO org. https://github.com/OpenTimelineIO/otio-fcp-adapter.

Working on adapter extraction based on the "extract_adapters" branch

markreidvfx commented 1 year ago

Can we merge the AAF adapter extraction https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1348 into the extract_adapters branch too?

apetrynet commented 1 year ago

Mark's PR is merged into the "extract_adapters" branch.

apetrynet commented 1 year ago

svg adapter repo ready https://github.com/OpenTimelineIO/otio-svg-adapter svg adapter extracted from "extract_adapters" branch

rosborne132 commented 1 year ago

I'll be working on moving the maya adapter.

rosborne132 commented 1 year ago

maya_sequencer adapter repo is ready: https://github.com/OpenTimelineIO/otio-maya-sequencer-adapter maya_sequencer adapter has been extracted from "extract_adapters" branch.

rosborne132 commented 1 year ago

I'll be working on moving the fcpxml adapter.

rosborne132 commented 1 year ago

fcpx_xml adapter repo is ready: https://github.com/OpenTimelineIO/otio-fcpx-xml-adapter fcpx_xml adapter has been extracted from "extract_adapters" branch.

rosborne132 commented 1 year ago

I'll be working on moving the hls_playlist adapter.

rosborne132 commented 1 year ago

hls_playlist adapter repo is ready for transfer here

Also the PR to extract the adapter from core is ready but is blocked by the follow issue

In the meantime while this is being resolved, I'm going to start work on the ale plugin.

rosborne132 commented 1 year ago

extract adapters repos have been merged and repos have been transferred. otio-ale-adapter otio-hls-playlist-adapter

rosborne132 commented 1 year ago

Just transferred the burnins adapter I will be starting on the final adapter xges.

ssteinbach commented 1 year ago

Updated title and comment to reflect what I see in the branch.

ssteinbach commented 1 year ago

Pinging this thread as we've updated this issue following conversation today. Comments welcome.

JeanChristopheMorinPerso commented 1 year ago

OK, I took some time to read the new proposal. Here are some thoughts:

These are the questions/comments I came up with when reading the new proposal. I think it's heading in the right direction, but some clarification will help us have a clearer idea. We've come a long way already and I'm excited to see this effort getting closer and closer to completion!

jminor commented 1 year ago

The main difference I see in the revised proposal is: old: opentimelineio = opentimelineio-core + pluginA, pluginB, etc. new: opentimelineio-plugins = opentimelineio + pluginA, pluginB, etc.

So there are still the same packages (umbrella = core + many plugins), but their names are different, such that "opentimelineio" is just the core rather than the umbrella. Am I understanding that right?

My only concern with that is that folks who are already using "pip install opentimelineio" will upgrade and lose their adapter plugins, get confused, and then discover that they need to "pip install opentimelineio-plugins" to get back to the functionality they had. For casual users this may be confusing and disruptive.

Can you clarify the intent of the revision? I missed the discussion on this.

ssteinbach commented 1 year ago

Thanks for the good questions!

  • What version scheme should opentimelineio-plugins use? I'm guessing CalVer could work.

That would be fine. I suspect that this package won't change all that much, since a change would be a plugin being added or removed, or the main repo being releasing a new version.

    • Create pypi project for OpenTimelineIO-Plugins
    • register pypi packages for the adapter plugin projects <- at least for the ones that we plan on having the umbrella project include

    What the difference between these two points?

One repo/project is named OpenTimelineIO-Plugins (the project/package for the umbrella set), each individual plugin repository also needs to be versioned and released on pypi (the repositories have been largely created I believe, but AFAIK they haven't been pushed to PyPI yet).

  • recommend a pinning strategy for how the adapters should handle their version dependencies

    I would personally advocate for no pinning. IMO, pinning is rarely that useful if we compare with less intrusive methods. I'm almost tempted to say we should let the dependencies dictate which version works. For example we could decide to include a plugin, A, that can work with otio>=0.15.0 and have another plugin, B, depend on otio>=0.16.0. This would mean that opentimelineio-plugins would require 0.16.0. The main idea is that anyway we'll need to define OpenTimelineIO as a dependency on every single plugin individually anyway.

I don't think we had something specific in mind. There was some concern about preventing conflicting version pinning (one plugin needs > some version, some other plugin hasn't been updated and says <= some version). I'm sure there is wisdom and experience in this space that we can draw on and if you have a perspective, I'd love to hear it!

  • add version freezing to release script for parent projects

    What does this mean?

In the release system for OTIO there is a script that handles freezing and unfreezing various versions. If we're going to freeze the OpenTimelineIO-Plugins repository on OpenTimelineIO versions, then we should have the release script handle that. https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/41ceb581d95d352afa04b07250211030880e9762/maintainers/freeze_ci_versions.py

  • OpenTimelineIO-Plugins project pins to a specific OpenTimelineIO project, corresponding to the current release, using the == constraint

    The pain question I have is what does pinning == brings on the table. Or in other words, what problem(s) is it solving?

You might be right! This was trying to tie a set of plugins to a "known compatible" version of OTIO. If you need adapter X and it is in the plugin set for OTIO v0.15 you can roll back or target a version of opentimelineio for interchange that has the adapter support you need.

  • How will users install the plugins? Will we add an extra on OpenTimelineIO (for example pip install opentimelineio[plugins])? Or we'll just document that they can pip install opentimelineio-plugins to get access to our vetted plugins?

Our discussion was to document and promote pip install opentimelineio-plugins, following in the "-extras" pattern that other package ecosystems use for core + plugins.

  • A topic I talked about before is how can we make the plugins discoverable to our users. If we start to have more and more plugins (which won't necessarily be included in the plugins package), how can we make it so that it's easy for our users to discover new/existing plugins, etc. We have https://opentimelineio.readthedocs.io/en/stable/tutorials/feature-matrix.html. Do we keep it in its current form? If so, how do we make sure it's kept up-to-date (maybe there is no easy way to do that)?

My only concern with that is that folks who are already using "pip install opentimelineio" will upgrade and lose their adapter plugins, get confused, and then discover that they need to "pip install opentimelineio-plugins" to get back to the functionality they had. For casual users this may be confusing and disruptive.

Answering these two together. This is definitely an issue with this approach, but our thought was the long term benefit of this strategy is worth it.

jminor commented 1 year ago

Thanks for the clarifications. Maybe we could mitigate the confusion/disruption of missing plugins by having the adapter system print a helpful message when an unsupported format is encountered: "No adapter plugin for format 'xyz' was found. Try 'pip install opentimelineio-plugins' and/or see https://.../adapters for details."

jminor commented 1 year ago

Regarding pinning to versions... We should consider whether the opentimelineio-plugins package is responsible for vetting the dependent plugins at all. If we leave it completely unpinned, then a change to any of the plugins which introduces a security problem, or needs a new/old version of the OTIO core, could cause problems. Maybe a minimal test case for each adapter, and dependabot could provide a minimal vetting, and a mechanism for knowing if/when changes occur?

apetrynet commented 1 year ago

Thanks for the clarifications. Maybe we could mitigate the confusion/disruption of missing plugins by having the adapter system print a helpful message when an unsupported format is encountered: "No adapter plugin for format 'xyz' was found. Try 'pip install opentimelineio-plugins' and/or see https://.../adapters for details."

This should be fairly easy to implement. There's already an error message we can update right? Should I add this to the "remove contrib" PR or create a new one? Probably makes sense to do this in a separate PR when we land the package setup.

JeanChristopheMorinPerso commented 1 year ago
  • recommend a pinning strategy for how the adapters should handle their version dependencies

I would personally advocate for no pinning. IMO, pinning is rarely that useful if we compare with less intrusive methods. I'm almost tempted to say we should let the dependencies dictate which version works. For example we could decide to include a plugin, A, that can work with otio>=0.15.0 and have another plugin, B, depend on otio>=0.16.0. This would mean that opentimelineio-plugins would require 0.16.0. The main idea is that anyway we'll need to define OpenTimelineIO as a dependency on every single plugin individually anyway.

I don't think we had something specific in mind. There was some concern about preventing conflicting version pinning (one plugin needs > some version, some other plugin hasn't been updated and says <= some version). I'm sure there is wisdom and experience in this space that we can draw on and if you have a perspective, I'd love to hear it!

Upper bounds (<, <=) and == cause conflicts. Lower bounds (>, >=) won't. We can easily make sure that all plugins included in OpenTimelineIO-Plugins don't use upper bounds. It's fine if they use lower bounds (>, >=) and even probably recommended.

In the release system for OTIO there is a script that handles freezing and unfreezing various versions. If we're going to freeze the OpenTimelineIO-Plugins repository on OpenTimelineIO versions, then we should have the release script handle that. https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/41ceb581d95d352afa04b07250211030880e9762/maintainers/freeze_ci_versions.py

I don't think this will be necessary for the plugins. Baking is handy when something is compiled, etc, but for pure python packages it probably just adds unnecessary overhead.

  • OpenTimelineIO-Plugins project pins to a specific OpenTimelineIO project, corresponding to the current release, using the == constraint

The pain question I have is what does pinning == brings on the table. Or in other words, what problem(s) is it solving?

You might be right! This was trying to tie a set of plugins to a "known compatible" version of OTIO. If you need adapter X and it is in the plugin set for OTIO v0.15 you can roll back or target a version of opentimelineio for interchange that has the adapter support you need.

"pain" was a typo, in case it matters. I wanted to say "main". But I see you point. In all cases, the OpenTimelineIO-Plugins package will be mainly used by new users and I anticipate that more advanced users will not use it because they'll probably one to mix and match plugins versions and plugins themselves.

Our discussion was to document and promote pip install opentimelineio-plugins, following in the "-extras" pattern that other package ecosystems use for core + plugins.

Works for me. Should an item be added in the issue description?

My only concern with that is that folks who are already using "pip install opentimelineio" will upgrade and lose their adapter plugins, get confused, and then discover that they need to "pip install opentimelineio-plugins" to get back to the functionality they had. For casual users this may be confusing and disruptive.

Answering these two together. This is definitely an issue with this approach, but our thought was the long term benefit of this strategy is worth it.

  • what pip install opentimelineio provides today is still a CLI only interface to the main tool (otioconvert) and a library, so folks who are using pip install opentimelineio have some kind of baseline savvyness
  • for integrations that want the adapters, their instructions on how to get OTIO can be updated to get that package or they can switch their pip dependency to point at the -plugins project if they're installed via pip
  • the OpenTimelineIO project reflects the main repository clearly it reinforces our goal that over time the adapters give way to native integrations
  • ...and that the adapters themselves are provided without warranty by the main project

I think it's a totally valid concern @jminor. But I think a good deal of documentation, a section in the release notes and good communication (in the release announcements, etc) should solve that concern IMO, or at least help.

reinecke commented 1 year ago

With help from @JeanChristopheMorinPerso and @timlehr we were able to:

Next Steps:

This should make it easy for users to test things out and give feedback.

apetrynet commented 1 year ago

Once we land the "extract_adapters" branch we need to update each adapters "ci.yaml" file to run tests against the main and tagged branches. Perhaps we should add this as a task in the issue description?

reinecke commented 5 months ago

The work for this ticket is done, any further work can be handled in follow-on issues.