search

salu133445 / muspy

A toolkit for symbolic music generation
https://salu133445.github.io/muspy/
MIT License
427 stars 49 forks source link

Adjust end time, not duration in adjust_time #37

Closed cifkao closed 3 years ago

cifkao commented 3 years ago

We want func to be applied to absolute timestamps, not durations.

cifkao commented 3 years ago

It's not easy to get this right...

It should be OK now. Not sure how to fix the coverage. (We're not testing Chord because there are no chords in the test data.)

salu133445 commented 3 years ago

Looks like a bug to me. Once time is adjusted, end (a property that returns time + duration) is no longer accurate.

However, time and duration are two different attributes, so we should handle them separately if possible for consistency reason. It can be quite tricky to adjust them independently due to potential accumulations of rounding errors.

cifkao commented 3 years ago

It doesn't seem to be possible to me to adjust them separately without caching the old time value somewhere... Or making sure duration gets adjusted first, which would be quite fragile.

I think maybe classes like Note and Chord should just override adjust_time instead of _adjust_time. That way the function will get called only once for the whole object and interactions between attributes can be handled easily.

cifkao commented 3 years ago

How about this?

salu133445 commented 3 years ago

Merged. Thank you!