Skip to content

Also merge spanners when calling stream.Score.partsToVoices#1796

Merged
mscuthbert merged 4 commits intocuthbertLab:masterfrom
guang-yng:master
Jun 28, 2025
Merged

Also merge spanners when calling stream.Score.partsToVoices#1796
mscuthbert merged 4 commits intocuthbertLab:masterfrom
guang-yng:master

Conversation

@guang-yng
Copy link
Contributor

What does this PR do?

When calling stream.Score.partsToVoices, only notesAndRests are merged into the new part. We can also merge the spanners from those parts into the new one.

@coveralls
Copy link

coveralls commented Jun 20, 2025

Coverage Status

coverage: 92.996% (+0.004%) from 92.992%
when pulling b32f683 on guang-yng:master
into f2e09f1 on cuthbertLab:master.

@mscuthbert
Copy link
Member

Hi guang-yng -- thanks for the contribution -- I'm looking forward to being able to merge this. Make sure that spanner is not used as a variable since it overshadows the name of the module (sp is what I usually use); use an editor that follows .editorconfig formats so you don't break music21 format; and most importantly please add tests to show that the spanners are collected.

What about spanners within the part's measures? Should we be collecting them? In which case

for sp in p[spanner.Spanner]:

might be the better loop variable.

@guang-yng
Copy link
Contributor Author

Hi @mscuthbert , thanks for the quick review! I’ll make the fixes and add test cases. Also, do you think we should merge Dynamic elements in the measures as well? Currently, only Note, Chord, and Rest are being merged. I’d be happy to include that too. Thanks again!

@mscuthbert
Copy link
Member

For now, one PR = one feature. We can do dynamics after you have a successful merge etc.

@guang-yng
Copy link
Contributor Author

Hi @mscuthbert, just checking in—would it be okay to merge this PR now?

@mscuthbert mscuthbert merged commit df1260e into cuthbertLab:master Jun 28, 2025
8 checks passed
@mscuthbert
Copy link
Member

Thanks for the bump!

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