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.

Sign up for GitHub

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

Repo improvements #2

Merged
merged 13 commits into from
Oct 3, 2022
Merged

Repo improvements #2

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8909275
Add code of conduct and contributors docs
markreidvfx Aug 4, 2022
47d734e
Fix badge url
markreidvfx Aug 4, 2022
d3e4942
Add issue and pull request templates
markreidvfx Aug 4, 2022
bdb07c1
Pip install from pull request
markreidvfx Aug 4, 2022
6afdc23
Add Codecov support
markreidvfx Aug 4, 2022
d441156
Bump sphinx from 4.5.0 to 5.1.1
markreidvfx Aug 7, 2022
6e6b6e5
Add dependabot
markreidvfx Aug 7, 2022
4f4b000
Improvements to README
markreidvfx Aug 10, 2022
482374a
Don't ignore deprecation warnings
markreidvfx Aug 29, 2022
80f0471
Remove compiler info bit and fix some typos
markreidvfx Aug 29, 2022
c51115d
Use markdown instead of RST style headers
markreidvfx Aug 29, 2022
bb39d85
Add a few missing contributors
markreidvfx Sep 3, 2022
c81e3d0
Remove version numbers since they might change often
markreidvfx Sep 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions .github/ISSUE_TEMPLATE/bug_report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
name: Bug report
about: Problem or regression with an existing feature
title: ''
labels: bug
assignees: ''

---

Before opening a bug report, please search open and closed issues to see if the problem has already been addressed. If there is such a report, and your issue is not adequately addressed there, please reference the issues in your new report.

## Bug Report

Please pick one of the following categories:

### Build Problem

Please provide your complete command line invocation, and attach a log of the console output.

### Incorrect Functionality and General Questions

Describe the issue here.

## To Reproduce

1. Operating System
2. Python version
3. Example snippet that demonstrates the issue - if it's a build issue,
4. OpenTimelineIO release version or commit hash
5. Compiler information:
on Mac, type `clang -v`, and paste the results here
on Linux, type `gcc -v`, and paste the results here
on Windows, type `cl`, and paste the results here.
If you are unable to determine your compiler via these commands, please indicate that here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's relevant here since pyaaf2 is pure Python? Or potentially we keep it here just in case someone simply installs otio-aaf-adapter without actually installing OTIO (which would mean OTIO would be installed by otio-aaf-adapter)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can get rid of this, I just missed it when copying the file over from the main OTIO repo


## Expected Behavior

Description of the expected behavior.

## Screenshots

If applicable, add screenshots to help explain your problem.

## Logs

If applicable, attach a conplete console log, that show a reproduction of the problem entirely.

## Additional Context

Add any other context about the problem here.
20 changes: 20 additions & 0 deletions .github/ISSUE_TEMPLATE/feature_request.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
name: Feature request
about: Suggest a new feature for OpenTimelineio
title: ''
labels: ''
assignees: ''

---

## Feature Request

Is this a New Feature, or a Proposed Change to existing Behavior?

## Description

Describe the new feature, or describe the modification to existing behavior, and provide context as necessary to help with understanding how the feature relates to a workflow or functionality.

## Context

Add any other context here, such as simulated output, or screenshots, that might help clarify the request.
22 changes: 22 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
**Link the Issue(s) this Pull Request is related to.**

Each PR should link at least one issue, in the form:

```Fixes #123```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```Fixes #123```
Fixes #123

I guess that was copied from OTIO repo. Using triple backquotes here will lead to Fixes #123 which GitHub won't be able to understand and so won't link the PR to the issue. Eventually it also needs to be fixed in OTIO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this was just copied from the main OTIO repo.


Use one line for each Issue. This allows auto-closing the related issue when the fix is merged.

**Summarize your change.**

Describe the reason for the change.

Add a list of changes, and note any that might need special attention during review.

**Reference associated tests.**

If no new tests are introduced as part of this PR, note the tests that are providing coverage.

<!--
Important: If this is your first contribution to OpenTimelineIO, you will need to submit a Contributor License Agreement. For a step-by-step instructions on the pull request process, see
https://github.com/AcademySoftwareFoundation/OpenTimelineIO/tree/main/CONTRIBUTING.md
-->
Comment on lines +19 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this stay here? Will CLA/CCLA be required for the adapters repos?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is my assumption that it is needed for officially supported adapters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed the pros/cons among the TSC and decided that yes, we would like to have the same CLA (and DCO) requirements for repos in the OpenTimelineIO GitHub organization. That will ensure that all the approvals and such are uniform between these and the core repo in the ASWF org. If a particular repo needs/wants to follow different rules, it can just live in a different org.
Does that sound reasonable to you two @markreidvfx and @JeanChristopheMorinPerso ?

Copy link
Member Author

@markreidvfx markreidvfx Sep 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to me!
I don't think DCO has been activated for this repo yet, I believe its somewhere in the repo settings.
Having EasyCLA available for plugin repos at some point would be great too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no problem with DCO and CLAs in general, but I do find that they are kind of a barrier of entry in some cases. I find that it's a lot of bureaucracy to contribute to an adapter. But I guess there is some legal questions regarding adapters that are moved out of the OTIO repo and stuff like that? Anyway, if the TSC decided to go with CLAs+DCO, then that's the way it is. I'll still happily sign the CLAs and contribute :)

13 changes: 13 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
version: 2
updates:

- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "daily"

- package-ecosystem: "pip"
directory: "/"
schedule:
day: "monday"
interval: "weekly"
15 changes: 14 additions & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
name: Run tests

env:
GH_COV_PY: 3.7
GH_COV_OS: ubuntu-latest
GH_DEPENDABOT: dependabot

on:
push:
branches: [main]
Expand Down Expand Up @@ -84,7 +89,7 @@ jobs:
run: |
python -m pip install --upgrade pip
pip install pytest pytest-cov wheel -V pyaaf2==1.4.0
pip install git+https://github.com/markreidvfx/OpenTimelineIO@remove_aaf_adapter_v1
pip install git+https://github.com/AcademySoftwareFoundation/OpenTimelineIO.git@refs/pull/1348/head
# pip install pytest pytest-cov -V pyaaf2==1.4.0 -V OpenTimelineIO==0.14.1

- name: Run Unit Tests
Expand All @@ -93,6 +98,14 @@ jobs:
python -m pip install dist/*.whl --no-index
pytest -v tests

- name: Upload coverage to Codecov
if: matrix.python-version == env.GH_COV_PY && matrix.os == env.GH_COV_OS && github.actor != env.GH_DEPENDABOT
uses: codecov/codecov-action@v3
with:
flags: unittests
name: otio-aaf-adapter-codecov
fail_ci_if_error: true

latest-release:
needs: test-otio-git
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions CODE_OF_CONDUCT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/main/CODE_OF_CONDUCT.md
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/main/CONTRIBUTING.md
18 changes: 18 additions & 0 deletions CONTRIBUTORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
This plugin is part of the OpenTimelineIO project, whose contributors are
listed at this URL:
https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/main/CONTRIBUTORS.md

This is a list of people who have contributed code to the
otio-aaf-adapter project, sorted alphabetically by first name.

If you know of anyone missing from this list, please contact us:
https://lists.aswf.io/g/otio-discussion

* Eric Reinecke ([reinecke](https://github.com/reinecke))
* Josh Burnell ([JoshBurnell](https://github.com/JoshBurnell))
* Joshua Minor ([jminor](https://github.com/jminor))
* Julian Yu-Chung Chen ([jchen9](https://github.com/jchen9))
* Mark Reid ([markreidvfx](https://github.com/markreidvfx))
* Stefan Schulze ([stefanschulze](https://github.com/stefanschulze))
* Stephan Steinbach ([ssteinbach](https://github.com/ssteinbach))
* Tim Lehr ([timlehr](https://github.com/timlehr))
Copy link
Member

@jminor jminor Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add these people also?

* Shahbaz Khan ([shahbazk8194](https://github.com/shahbazk8194))
* Freeson Wang ([freesonluxo](https://github.com/freesonluxo))
* Andrew Moore ([andrewmoore-nz](https://github.com/andrewmoore-nz))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do!

29 changes: 28 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ OpenTimelineIO Advanced Authoring Format (AAF) Adapter

[![Supported VFX Platform Versions](https://img.shields.io/badge/vfx%20platform-2018--2021-lightgrey.svg)](http://www.vfxplatform.com/)
![Supported Versions](https://img.shields.io/badge/python-2.7%2C%203.7%2C%203.8%2C%203.9%2C%203.10-blue)
[![Run tests](https://github.com/markreidvfx/otio-aaf-adapter/actions/workflows/ci.yaml/badge.svg)](https://github.com/markreidvfx/otio-aaf-adapter/actions/workflows/ci.yaml)
[![Run tests](https://github.com/markreidvfx/otio-aaf-adapter/actions/workflows/ci.yaml/badge.svg)](https://github.com/OpenTimelineIO/otio-aaf-adapter/actions/workflows/ci.yaml)

Overview
--------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Overview
--------
# Overview

I would recommend using markdown headers. It will render more nicely.


This project is a [OpenTimelineIO](https://github.com/AcademySoftwareFoundation/OpenTimelineIO) adapter for reading and writing Advanced Authoring Format (AAF) files.
This adapter was originally included with OpenTimelineIO as a contrib adapter. It is in the process of being separated into this project to improve maintainability and reduced the dependencies of both projects.

Feature Matrix
--------------
Expand All @@ -24,7 +29,17 @@ Feature Matrix
| Color Decision List | ✖ | ✖ |
| Image Sequence Reference | ✖ | ✖ |

Requirements
------------

* [OpenTimelineIO](https://github.com/AcademySoftwareFoundation/OpenTimelineIO) > 14.1 (latest git version currently recommended)
* [pyaaf2](https://github.com/markreidvfx/pyaaf2) >= 1.4.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to list the requirements? They are already set in the setup.py. I find that redundant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't like having to dig into a project files to find such basic information, but I can remove it if preferred.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of making it clear that you need these two packages (esp pyaaf2), but let's omit the version info here, since that may change often.

Copy link
Member Author

@markreidvfx markreidvfx Sep 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, I'll remove the version numbers.



Licensing
---------

This repository is licensed under the [Apache License, Version 2.0](LICENSE.md).

Testing for Development
-----------------------
Expand All @@ -36,3 +51,15 @@ pip install -e .
# Test adapter
otioconvert -i some_timeline.aaf -o some_timeline.ext
```

If you are using a version of OpentimelineIO that still has the AAF contrib adapter you may need to add the path of [plugin_manifest.json](./src/otio_aaf_adapter/plugin_manifest.json) to your `OTIO_PLUGIN_MANIFEST_PATH` [environment variable.](https://opentimelineio.readthedocs.io/en/latest/tutorials/otio-env-variables.html) This should override the contrib version.

Contributions
-------------

If you have any suggested changes to the otio-aaf-adapter,
please provide them via [pull request](../../pulls) or [create an issue](../../issues) as appropriate.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really works? If so I just learned something!

Copy link
Member Author

@markreidvfx markreidvfx Aug 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it does, saw some other repos doing it. Was very surprised too that it works!


All contributions to this repository must align with the contribution
[guidelines](https://opentimelineio.readthedocs.io/en/latest/tutorials/contributing.html)
of the OpenTimelineIO project.
2 changes: 2 additions & 0 deletions codecov.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fixes:
- "*/site-packages/::src/"
6 changes: 3 additions & 3 deletions docs/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
sphinx==4.5.0
readthedocs-sphinx-ext==2.1.5 # ??
sphinx==5.1.1
readthedocs-sphinx-ext==2.1.8
sphinx-rtd-theme
myst-parser==0.17.2
myst-parser==0.18.0
2 changes: 1 addition & 1 deletion setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ ignore =
W504

[tool:pytest]
addopts = --cov=./ -W ignore::DeprecationWarning
addopts = --cov=otio_aaf_adapter -W ignore::DeprecationWarning
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
addopts = --cov=otio_aaf_adapter -W ignore::DeprecationWarning
addopts = --cov=otio_aaf_adapter

It is generally better if CI logs (or even fails, -W error::DeprecationWarning) when DeprecationWarnings are encountered.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is like that in the template repo, but I agree

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could try turning that off to see if it passes. I suspect that since we're running against such a wide range of Python versions that some deprecations are unavoidable...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already turned it off ignore and all the tests still pass. I didn't try -W error::DeprecationWarning though, I thought that would be overkill for now.


[bdist_wheel]
# Let a wheel be compatible with both Python 2 and 3
Expand Down
3 changes: 3 additions & 0 deletions tests/test_aaf_adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
AAFValidationError
)

# module needs to be imported for code coverage to work
import otio_aaf_adapter.adapters.advanced_authoring_format # noqa: F401

try:
# python2
import StringIO as io
Expand Down