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

new SchemaDef plugin type to define site-specific schemas #342

Merged
merged 10 commits into from Oct 24, 2018

Conversation

Projects
None yet
4 participants
@peachey
Collaborator

peachey commented Oct 11, 2018

This PR adds a new SchemaDef plugin type. A SchemaDef plugin can define one or more new OTIO schemas (or schemata if you prefer) which define first-class serializable OTIO objects and can be upgraded based on changes to the schema version number just like other OTIO schema-based objects.

One possible use case is to define new schemas for studio-specific metadata objects which can be included in the clip "metadata" field. For instance, our internal media linker plugin uses a few Pixar-specific schemas to provide metadata values which can be automatically upgraded when the schema version number increases. Those Pixar-specific metadata schemas are defined in a single SchemaDef Python plugin and included by our plugin_manifest.json.

@codecov-io

This comment has been minimized.

codecov-io commented Oct 11, 2018

Codecov Report

Merging #342 into master will increase coverage by 0.18%.
The diff coverage is 89.33%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #342      +/-   ##
========================================
+ Coverage   73.82%    74%   +0.18%     
========================================
  Files          59     62       +3     
  Lines        5352   5417      +65     
========================================
+ Hits         3951   4009      +58     
- Misses       1401   1408       +7
Impacted Files Coverage Δ
opentimelineio/__init__.py 100% <ø> (ø) ⬆️
opentimelineio/schema/missing_reference.py 100% <ø> (ø) ⬆️
opentimelineio/core/__init__.py 100% <100%> (ø) ⬆️
opentimelineio/core/type_registry.py 100% <100%> (+3.03%) ⬆️
opentimelineio/core/serializable_object.py 91.37% <100%> (+0.3%) ⬆️
opentimelineio/schemadef/__init__.py 100% <100%> (ø)
opentimelineio/core/unknown_schema.py 100% <100%> (ø)
opentimelineio/plugins/python_plugin.py 94.73% <100%> (ø) ⬆️
opentimelineio/schema/__init__.py 100% <100%> (ø) ⬆️
opentimelineio/schema/schemadef.py 73.68% <73.68%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a13284e...473bf62. Read the comment docs.

@peachey

This comment has been minimized.

Collaborator

peachey commented Oct 19, 2018

I think the test failures from Travis CI are actually the ones that will be fixed when ssteinbach's PR #347 is merged.

@ssteinbach ssteinbach added the WIP label Oct 22, 2018

@ssteinbach ssteinbach changed the title from new SchemaDef plugin type to define site-specific schemas to [WIP] new SchemaDef plugin type to define site-specific schemas Oct 22, 2018

@peachey peachey changed the title from [WIP] new SchemaDef plugin type to define site-specific schemas to new SchemaDef plugin type to define site-specific schemas Oct 23, 2018

@jminor jminor removed the WIP label Oct 23, 2018

@jminor jminor requested review from ssteinbach and jminor Oct 23, 2018

peachey added some commits Oct 11, 2018

Explicitly include our source files in MANIFEST.in
Travis-CI is having some problems reconciling the file list between
sdist and the repo and suggested adding this line:
recursive-include opentimelineio *
so I'm adding it to see if that works.
The particular problem is that it is not picking up one of my latest
additions, namely the schemadef directory and the __init__.py file
in it.  That's the only source file in that directory -- maybe that's
the problem?
add UnknownSchema object to pass through undefined schemas
Old behavior was to through an exception if any object being read
(or created by instance_from_schema) had an unregistered schema name.
New behavior is to replace the schema of such objects with the
UnknownSchema schema type and save the original schema label in a
field called "UnknownSchemaOriginalLabel".  Such objects are then
available within the OTIO data structure.  The new property
"is_unknown_schema" will be True for these objects and False for all
other SerializableObjects.  When an UnknownSchema is serialized,
the json_serializer will replace the UnknownSchema label with the
original label, so that (for instance) otiocat will pass such
unregistered objects through unchanged.  This is part of the schemadef
project because schemadef plugins imply that we will start to see
site or studio-specific OTIO schema types, and it would be more polite
to pass these through quietly if you don't understand them.
@ssteinbach

This looks great! I think the write-a-schemadef.md needs to be added to the documentation index in docs/index.rst, and I had a few other small things I saw in the tests. Thanks for submitting this!

Show resolved Hide resolved opentimelineio/schema/schemadef.py
# Our test manifest should have been loaded, including
# the example_schemadef.
peculiar_value = "something One-derful"
try:

This comment has been minimized.

@ssteinbach

ssteinbach Oct 23, 2018

Member

You can use the with assertRaises syntax to do this, see:

with self.assertRaises(TypeError):

This comment has been minimized.

@peachey

peachey Oct 24, 2018

Collaborator

The intent here was to check that it DOESN'T raise NotSupportedError, whereas self.assertRaises checks that it DOES raise the exception, I think. But this condition likely could be removed anyway, since I took out the code in instance_from_schema that could raise this exception. Instead now it will return an UnknownSchema object, so I should check for that instead, I guess.

This comment has been minimized.

@peachey

peachey Oct 24, 2018

Collaborator

Oh, and it already does check that by verifying the type of the created class, so if it is UnknownSchema rather than exampleSchemaDef, the type check will fail.

self.assertEqual(str(type(example)), EXCLASS)
self.assertEqual(example.exampleArg, peculiar_value)
# Repeat test using the direct class definition method:

This comment has been minimized.

@ssteinbach

ssteinbach Oct 23, 2018

Member

Could make this a second test method, since you've already set up the values.

This comment has been minimized.

@peachey

peachey Oct 24, 2018

Collaborator

Yes, good idea.

Show resolved Hide resolved tests/test_unknown_schema.py

@ssteinbach ssteinbach added this to the Public Beta 9 milestone Oct 24, 2018

@ssteinbach ssteinbach merged commit 9459b2c into PixarAnimationStudios:master Oct 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jminor jminor referenced this pull request Nov 1, 2018

Closed

Unregistered schema support #326

@ssteinbach ssteinbach referenced this pull request Nov 2, 2018

Closed

"SchemaDef" Plugins #341

alatdneg added a commit to alatdneg/OpenTimelineIO that referenced this pull request Nov 13, 2018

new SchemaDef plugin type to define site-specific schemas (PixarAnima…
…tionStudios#342)

* new SchemaDef plugin type to define site-specific schemas
* add missing _update_plugin_source for pkg_resources; force schemadefs to load
* add SchemaDef plugin test
* add method to extend all three manifest plugin lists at once
* cleaner save/restore of existing _MANIFEST
* schemadef plugin classes added to otio.schemadef namespace
* Explicitly include our source files in MANIFEST.in

Travis-CI is having some problems reconciling the file list between
sdist and the repo and suggested adding this line:
recursive-include opentimelineio *
so I'm adding it to see if that works.
The particular problem is that it is not picking up one of my latest
additions, namely the schemadef directory and the __init__.py file
in it.  That's the only source file in that directory -- maybe that's
the problem?

* add UnknownSchema object to pass through undefined schemas

Old behavior was to through an exception if any object being read
(or created by instance_from_schema) had an unregistered schema name.
New behavior is to replace the schema of such objects with the
UnknownSchema schema type and save the original schema label in a
field called "UnknownSchemaOriginalLabel".  Such objects are then
available within the OTIO data structure.  The new property
"is_unknown_schema" will be True for these objects and False for all
other SerializableObjects.  When an UnknownSchema is serialized,
the json_serializer will replace the UnknownSchema label with the
original label, so that (for instance) otiocat will pass such
unregistered objects through unchanged.  This is part of the schemadef
project because schemadef plugins imply that we will start to see
site or studio-specific OTIO schema types, and it would be more polite
to pass these through quietly if you don't understand them.

* improve docs and tests for schemadef and unknown schema
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment