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

Adding key signatures to generated chorales #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

napulen
Copy link
Contributor

@napulen napulen commented Sep 12, 2020

This PR adds key signatures to the generated chorales.

Currently, the generated score shows the accidentals in every note, and has a key signature of C / a:

image

It'd be better to output the key signature that corresponds to the key of the chorale:

image

Some artifacts are still happening with the accidentals. For example, the first B- shouldn't be printed. I am afraid that has to do with how music21 processes the score. Yet, I still think it would be good to add this change here, and maybe submit an issue in music21 related to the artifacts in accidentals. Maybe there is a more idiomatic way of encoding the chorales in music21 that works better with their routines for generating the score.

@ekzhang
Copy link
Owner

ekzhang commented Sep 16, 2020

Yeah, good point, I've had issues in the past re: key signatures as well. It seems like Music21 handles this somewhat inconsistently. I'll try to take a look.

@napulen
Copy link
Contributor Author

napulen commented Sep 16, 2020

My guess is that it has to do with the intermediate Measure objects that music21 adds to the representation. In many files I have parsed with music21, I remember I seeing the key signatures encoded at the beginning of Measure0 or Measure1.

I like your encoding strategy, because it separates the voices across the 2 staffs and its super compact code-wise. However, it doesn't have any measures and I imagined these are constructed automatically by music21 at some point. Maybe the library is struggling during that process, and having the intermediate Measure objects would fix this issue.

I still think this looks more like a music21 bug, maybe. I think it's logical to encode the notes/time signature and expect the library to handle the rest automatically.

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.

2 participants