Bundle fixes - #2060
Open
darbyjohnston wants to merge 5 commits into
Open
Bundle fixes#2060darbyjohnston wants to merge 5 commits into
darbyjohnston wants to merge 5 commits into
Conversation
A bundle is written with a version file, and read_otiod refused to read one without it. The adapter that wrote otiod bundles before this never wrote one, so every bundle made until now was refused -- and read_otioz asks for no such thing, so the two halves of the format disagreed. The file is still written. It is no longer required to read, and nothing is lost by that: its contents were never read, only its presence tested. Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
The policy used to be a string, and the documentation still gives the
commandline for one:
otioconvert -i in.otio -o out.otioz -A media_policy="AllMissing"
That has not worked since the policy became an enumeration. An adapter
argument off the commandline is always text, and the text went straight to
the enumeration, which does not take it. Neither spelling worked: not
AllMissing, which is documented, nor all_missing, which is what the
enumeration calls it. The policy could not be set from the commandline at
all.
The adapters now read either spelling, which is where it belongs: they are
the edge where untyped arguments arrive. The enumeration stays strict for
everyone who already has a value to pass, and a name that is neither says
so rather than failing as a type error further in.
Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
The Python adapter builds a ReadOptions and calls read_otiod, so the test walked the bindings into the same code the C++ test covers. Whether a bundle without a version file reads is settled there. Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
Both bundle adapters call the same conversion, and the otiod test already covers both spellings and a value that is neither. Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2060 +/- ##
==========================================
+ Coverage 83.52% 83.69% +0.17%
==========================================
Files 182 183 +1
Lines 13533 13552 +19
Branches 1255 1256 +1
==========================================
+ Hits 11303 11343 +40
+ Misses 2057 2036 -21
Partials 173 173
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While testing the new bundle support I found a couple of bugs:
version.txtfiles to .otiod bundles, so we shouldn't throw an error if it is missing. That way older .otiod files remain readable. The new C++ code does writeversion.txtfiles to both .otiod and .otioz bundles.🤖 Generated with Claude Code using Claude Opus 5