cuthbertLab / music21

music21 is a Toolkit for Computational Musicology
https://www.music21.org/
Other
2.13k stars 402 forks source link

stripTies() needs to bridge voices #1134

Open jacobtylerwalls opened 3 years ago

jacobtylerwalls commented 3 years ago

music21 version dev

Problem summary stripTies() should bridge voices between measures rather than just flattening entire parts. Undesired deletions can happen. This was a known issue; we haven't made this worse recently AFAICT.

Steps to reproduce

>>> p1 = converter.parse('tinyNotation: g2 g2 g2~ g2')
>>> p2 = converter.parse('tinyNotation: e2 e2~ e2 e2')
>>> s = stream.Score([p1, p2])
>>> merged = s.partsToVoices()
>>> merged.show()
>>> stripped = merged.stripTies()  # matchByPitch True or False yields same result
>>> stripped.show()

Expected vs. actual behavior Before stripping ties

Screen Shot 2021-09-17 at 9 39 03 AM

After stripping ties (inappropriate deletion)

Screen Shot 2021-09-17 at 9 39 22 AM
>>> stripped.show('t', addEndTimes=True)
{0.0 - 8.0} <music21.stream.Part 0x7fdbd16b9a90>
    {0.0 - 8.0} <music21.stream.Measure 1 offset=0.0>
        {0.0 - 0.0} <music21.clef.TrebleClef>
        {0.0 - 0.0} <music21.meter.TimeSignature 4/4>
        {0.0 - 4.0} <music21.stream.Voice 0>
            {0.0 - 2.0} <music21.note.Note G>
            {2.0 - 4.0} <music21.note.Note G>
        {0.0 - 8.0} <music21.stream.Voice 1>
            {0.0 - 2.0} <music21.note.Note E>
            {2.0 - 8.0} <music21.note.Note E>
    {4.0 - 8.0} <music21.stream.Measure 2 offset=4.0>
        {0.0 - 4.0} <music21.stream.Voice 0>
            {2.0 - 4.0} <music21.note.Note G>
        {0.0 - 4.0} <music21.stream.Voice 1>
            {2.0 - 4.0} <music21.note.Note E>
        {4.0 - 4.0} <music21.bar.Barline type=final>

More information We could match voices by .id or by sequence in the measure container. Matching by sequence will be easier after having a .sequence attribute in #915.

mscuthbert commented 3 years ago

Good problem diagnosis. Because it fixes an existing bug, it can go in the v7 release series also (milestone is still fine) -- no one is counting on this behavior.

jacobtylerwalls commented 3 years ago

Maybe we want a recurse(voicesFirst=True) argument that will match by id, and sort by whatever order those id's were first encountered. Linking a related issue from m21j.

jacobtylerwalls commented 3 years ago

Or, before falling back to id's and order of encountering them, we implement the VoiceSpanner idea from discussion on #915 and order it by order of the VoiceSpanners in the stream.

jacobtylerwalls commented 2 years ago

In terms of voices-first recursion, that would also solve this wrinkle (add test case when solved -- makeAccidentals created another F# even though tied):

Screen Shot 2022-02-18 at 2 51 24 PM