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

Sort exported notes by step when same midi_pitch #407

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

leleogere
Copy link

@leleogere leleogere commented Dec 19, 2024

In MusicXML export, the notes are currently sorted using their midi_pitch attribute. However, this can lead to inconsistent exports when the score contains notes that are enharmonic equivalents (such as D# and Eb, or B# and C), as the export will differ depending on the order the user inserts the notes in the Part object:

score.add(pt.score.Note("C", 4, 0), start=0, end=1)  # midi_pitch=60
score.add(pt.score.Note("B", 3, 1), start=0, end=1)  # midi_pitch=60
# will result in a different file than
score.add(pt.score.Note("B", 3, 1), start=0, end=1)  # midi_pitch=60
score.add(pt.score.Note("C", 4, 0), start=0, end=1)  # midi_pitch=60

This PR simply adds the step as a second key for the sorting. Therefore, notes are first sorted by midi_pitch, and when they are enharmonic equivalents they are sorted by step, so that the export file is consistent no matter the order the notes are added to the score.

This will solve a potential issue raised in the second message of #406.

@fosfrancesco fosfrancesco self-assigned this Dec 19, 2024
@fosfrancesco fosfrancesco added the bug Something isn't working label Dec 19, 2024
@fosfrancesco
Copy link
Member

An effective solution for the inconsistent ordering of enharmonic notes in the musicxml export.

@fosfrancesco fosfrancesco merged commit f4930b5 into CPJKU:develop Dec 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants