cuthbertLab / music21

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

Enhance kern grace note importing, fix durations of partially notated chords #1706

Open alexandermorgan opened 6 months ago

alexandermorgan commented 6 months ago

This pr is a rework of a previous pr that I accidentally closed when resyncing my fork, sorry if that's confusing. I've addressed the comments that were made on that previous pr. This enhances kern grace note imports to allow "qq" to mean an unslashed grace note, and for the unlinked duration to take that of the written note if one is provided. This makes the unlinked duration of imported kern grace notes behave the same everywhere else in music21. Also, for kern chords with durations only specified on the first note in the chord ("8C E G"), non-first notes will now reuse that duration instead of defaulting to a quarter note duration. I also added 4 tests to the spineParser.py file to cover all of this new import behavior.

This pr also adds types to a couple functions in spineParser.py. I had a bit of a hard time with the mypy types because my local mypy setup doesn't give the same exact output as the CI test does. Please be aware that I had to use the comment # type: ignore 4 times in this file so I hope that's not an issue.

Additionally, this pr separates duration processing into its own function, and decreases the overall import time for kern scores by ~12%. The speed up is primarily due to the following:

  1. Move duration parsing of notes to the beginning of hdStringToNote so that notes and rests can be instantiated with their correct duration from the beginning rather than being created and then having their duration replaced. This prevents an unused default duration from being created on each note and rest.
  2. Combine the normal duration and rational duration regexes into one. Likewise for note name and accidental searches.
  3. Place variables such tat duplicate operations are avoided.
  4. Comment out parsing that only leads to pass statements.

The previous version of this pr also included memoization and safe short-circuiting, but those were discarded in favor of simplicity. That earlier pr also replaced str.count('x') with if x in str where possible but that has already been addressed by this pr.

Looking quickly, this same basic idea of figuring out the duration part before creating a note could apply in tinyNotation, abc, and m21ToXml.py->GeneralObjectExporter.fromDuration/fromScale/fromDiatonicScale And a similar idea, but figuring out pitch first, might apply in lots of places.

coveralls commented 6 months ago

Coverage Status

coverage: 93.04% (+0.007%) from 93.033% when pulling 502740484af34d396d2eb720bdf14e23841fff07 on alexandermorgan:master into 3fae7a15d8e77186a43851e3eafab5c228c373e7 on cuthbertLab:master.