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

Making voices can disrupt makeAccidentals() #577

Closed
jacobtylerwalls opened this issue Jul 15, 2020 · 4 comments · Fixed by #578
Closed

Making voices can disrupt makeAccidentals() #577

jacobtylerwalls opened this issue Jul 15, 2020 · 4 comments · Fixed by #578

Comments

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Jul 15, 2020

Based on case reported by Igor Praznik, music21 list 3/3/20.

s = stream.Stream()
s.insert(key.Key('Gb'))
s.insert(0, note.Note('D-5'))
s.insert(0, note.Note('D-4'))
s.show()  # During export to xml, makeNotation(), makeVoices(), makeMeasures()

Screen Shot 2020-07-14 at 9 19 28 PM

makeNotation() calls makeAccidentals() per Measure object, but makeAccidentals() only inspects measure.notesAndRests, which is empty in this case because the notes have been put in voices already:

# NOTE: this may or may have sub-streams that are not being examined

Need to grab the voice objects somewhere, possibly just recursively calling makeAccidentals(), but there are going to be corner cases here with accidentals tied from previous measures. Update: I think this is only affecting the code path where measures have not been created yet. In that case, probably easier to just call makeAccidentals() before makeVoices() and makeMeasures(), since without measures to worry about, those corner cases go away.

@jacobtylerwalls jacobtylerwalls changed the title Calling makeNotation() on a Measure with Voices causes makeAccidentals() to have no effect Creating measures through makeNotation() causes makeAccidentals() to have no effect Jul 15, 2020
@jacobtylerwalls
Copy link
Member Author

Or, could even do this in makeVoices(), since the root of the problem is that notes are being yanked from Measure objects into Voices, something like:

if not returnObj.haveAccidentalsBeenMade():
    returnObj.makeAccidentals()

@jacobtylerwalls jacobtylerwalls changed the title Creating measures through makeNotation() causes makeAccidentals() to have no effect Making voices can disrupt makeAccidentals() Jul 15, 2020
@mscuthbert
Copy link
Member

Hmmm...it seems better to fix this in makeAccidentals or makeNotation, since I wouldn't expect makeVoices to affect accidental status. But not entirely sure.

@jacobtylerwalls
Copy link
Member Author

Agreed. The patch I linked just adds recurse() in makeAccidentals.

@mscuthbert
Copy link
Member

Fixed in #578 -- thanks!

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 a pull request may close this issue.

2 participants