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

MSPA 427 attributes #44

Merged
merged 28 commits into from
Dec 4, 2019
Merged

Conversation

nigelmegitt
Copy link
Collaborator

PR for removing unwanted elements.

Creates an ElementRemover node that takes a comma separated list of elements to be removed. This PR includes an example configuration script that removes the relevant set of metadata elements.

@nigelmegitt
Copy link
Collaborator Author

Self-review comments: needs a unit test for the ElementRemover, and needs some additional functionality to meet remaining requirements of the MSPA-427 ticket. Not assigning a reviewer until I've done those bits.

@nigelmegitt
Copy link
Collaborator Author

My plan is to do the remaining parts of the (BBC specific) ticket in the BBC specific code, in which case this is good to review. Will assign a reviewer!

@nigelmegitt nigelmegitt self-assigned this Nov 29, 2019
@nigelmegitt
Copy link
Collaborator Author

By the way, for review purposes, it might make sense to wait until #43 has been merged, since this branch is based off that PR's branch, and the deltas will be easier to understand.

danielthepope and others added 21 commits December 2, 2019 11:11
MSPA-728 ebu-tt-3 to ebu-tt-d conversion
Does not handle time conversion.
Sets `ebuttp:sequenceIdentifier` to the value of `tt/head/metadata/documentMetadata/documentIdentifier` if present, otherwise uses "TestConverter".
Resets the `conformsToStandard` to say it is EBU-TT-3 conformant.
Sets the `timeBase` to `media` whether you like it or not, but doesn't do any other conversions.
Also allows for a setting that specifies whether or not to use `ebuttm:documentIdentifier` element value in the input as the `ebuttp:sequenceIdentifier` attribute value in the output. Adds a test for this.
Unit test for this also added.
Tidy out some print statements that had made their way in there.
The docs in current state don't build properly because  autodoc throws a "cannot import name `EBUTT1EBUTT3Converter`" exception. This is possibly a problem with  circular `import` references, which should be resolved before we merge. This can be demonstrated by adding `from . import ebutt1_ebutt3` to `ebu_tt_live/bindings/converters/__init__.py` and running `python -m ebu_tt_live.bindings.converters` which shows a stack trace illustrating the issue.
@nigelmegitt
Copy link
Collaborator Author

nigelmegitt commented Dec 2, 2019

Rebased on tt1-to-tt3 branch after merge of #43, and some pep8 tidying done too. Ready for review.

* Don't process duplicate documents
* Don't process documents from more than one sequence
* Factor out element removal from sequence processing so we can have a returning method that can be called independently of `process_document` in case the node's functionality is needed without providing a concrete carriage mechanism.
* Move project-wide data to a single file so we can import it
* Import it into the doc generation `conf.py` and the `setup.py` - great, updating the version now only needs one line of code to change!
Copy link
Member

@danielthepope danielthepope left a comment

Choose a reason for hiding this comment

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

Not many things to fix ;)

ebu_tt_live/node/element_remover.py Show resolved Hide resolved
ebu_tt_live/documents/test/test_converters.py Outdated Show resolved Hide resolved
ebu_tt_live/node/test/test_element_remover.py Outdated Show resolved Hide resolved
ebu_tt_live/node/element_remover.py Outdated Show resolved Hide resolved
Copy link
Member

@danielthepope danielthepope left a comment

Choose a reason for hiding this comment

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

Fine now! Thanks

@nigelmegitt nigelmegitt merged commit 4f68942 into tt1-to-tt3-conversion Dec 4, 2019
@nigelmegitt nigelmegitt deleted the mspa-427-attributes branch December 4, 2019 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants