-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Script to increase c-clef octaves by 1;tested 94r #15
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,2962 @@ | |||
<?xml version='1.0' encoding='utf-8'?> | |||
<ns0:mei xmlns:ns0="http://www.music-encoding.org/ns/mei" meiversion="5.0.0-dev"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the namespace has changed from the original? thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is to do with how ElementTree writes XML, wherein it needs to represent namespace and creates a prefix; in this case ns0
(this is stemming from my very foundational understanding of how this works, so I may be mistaken). If you think its better to hold onto the original, adding something like ET.register_namespace('', 'http://www.music-encoding.org/ns/mei')
shouldn't be too tricky? Also might be the wrong way forward, keen to hear your instinct/vibe. I don't believe the altered namespace should alter the validity of the MEI, however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see now. Yeah, it seems cleaner to just do what was in the original which is have the mei namespace be the default.
@@ -0,0 +1,2962 @@ | |||
<?xml version='1.0' encoding='utf-8'?> | |||
<ns0:mei xmlns:ns0="http://www.music-encoding.org/ns/mei" meiversion="5.0.0-dev"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also looks like we lose the schema information that are in the original xml files. See
e2e-omr-resources/resulting_mei_files/Einsiedeln/Reviewed_once/CH-E_611_001r.mei
Lines 2 to 3 in 5e9aaf6
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://relaxng.org/ns/structure/1.0"?> | |
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes! Will more explicitly snag the header!
preserves file format - Fixes file duplication issue in output - Correctly increases octaves in C-clef sections - Maintains proper MEI header formatting - Preserves original XML structure
New test is done on 100v, removed other duplicate test files. |
Ok, I think this PR is basically ready to merge. Just a few things:
Happy for you to take these on @kyrieb-ekat, or I can update this PR with them. |
No description provided.