CPJKU / partitura

A python package for handling modern staff notation of music
https://partitura.readthedocs.io
Apache License 2.0
241 stars 18 forks source link

Faster and more Robust Kern Import #344

Closed manoskary closed 5 months ago

manoskary commented 9 months ago

Faster and more Robust Kern Import

Updates

STATISTICS

NOT implemented yet

codecov-commenter commented 9 months ago

Codecov Report

Attention: 98 lines in your changes are missing coverage. Please review.

Comparison is base (018f264) 84.96% compared to head (ef751af) 84.12%. Report is 1 commits behind head on develop.

Files Patch % Lines
partitura/io/importkern_v2.py 90.84% 37 Missing :warning:
partitura/score.py 42.85% 36 Missing :warning:
partitura/io/exportkern.py 84.93% 25 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #344 +/- ## =========================================== - Coverage 84.96% 84.12% -0.85% =========================================== Files 82 83 +1 Lines 14236 14569 +333 =========================================== + Hits 12096 12256 +160 - Misses 2140 2313 +173 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fosfrancesco commented 5 months ago

Ideally, it would be nice to have a lot of tests for the functions to fill rests since they can be a bit tricky. But since they are not the main topic of this branch, I would open another branch with only these tests, and eventual modifications to the functions.

manoskary commented 5 months ago

Ideally, it would be nice to have a lot of tests for the functions to fill rests since they can be a bit tricky. But since they are not the main topic of this branch, I would open another branch with only these tests, and eventual modifications to the functions.

Thank you for the review. I agree this is particularly important. The kern import uses only measure wise rest infilling, meaning it doesn't happen for the entire piece but only for certain measures where the number of voices changes. I noticed that there are some lines that do not have coverage right now and I will address this in the tests. For the rest infilling, I agree that a separate PR with tests and checks would be probably better and clearer. Once the current PRs are closed I will update the tests.