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

A few leadsheet-related fixes #1708

Merged

Conversation

gregchapman-dev
Copy link
Contributor

@gregchapman-dev gregchapman-dev commented May 6, 2024

MusicXML import: forward tags with voice should trigger the same behavior as note tags with voice.

I see this quite a bit in leadsheets where these are used to position chordsym changes within a single melody note. The result should be a second voice containing only hidden rests, but instead, they land in the measure with the notes, and they overlap.

…vior as note tags with voice.

I see this quite a bit in leadsheets where these are used to position chord changes within a melody note.
@gregchapman-dev
Copy link
Contributor Author

Huh. That test failure has nothing to do with this PR.

@gregchapman-dev gregchapman-dev changed the title MusicXML import: forward tags with voice should trigger the same behavior as note tags with voice. A few leadsheet-related fixes May 8, 2024
@gregchapman-dev
Copy link
Contributor Author

A second fix: a missing opFrac call in OffsetHierarchyFilter that I ran into.

@coveralls
Copy link

Coverage Status

coverage: 93.034% (+0.001%) from 93.033%
when pulling 57ed01f on gregchapman-dev:gregc/moreLeadSheetFixes
into 3fae7a1 on cuthbertLab:master.

Copy link
Member

@mscuthbert mscuthbert left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

self.voiceIndices.add(vIndex)
# it is a set, so no need to check if already there
# additional time < 1 sec per ten million ops.
for tagSearch in ('note', 'forward'):
Copy link
Member

Choose a reason for hiding this comment

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

Should we also be searching in <direction> tags? They're the only other thing that can contain a voice tag. But maybe unnecessary.

@@ -490,7 +490,7 @@ def __call__(self, e, iterator=None):
if not hasattr(iterator, 'iteratorStartOffsetInHierarchy'):
raise FilterException('Can only run OffsetHierarchyFilter on a RecursiveIterator')

offset = s.elementOffset(e) + iterator.iteratorStartOffsetInHierarchy
offset = opFrac(s.elementOffset(e) + iterator.iteratorStartOffsetInHierarchy)
Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@mscuthbert mscuthbert merged commit 4716deb into cuthbertLab:master Jun 13, 2024
7 checks passed
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