Add optional attributes to tuplets related to their time modification

[leleogere]

As I explained in #405, I thought that tuplets objects were missing some information about the rhythmic modifications they induce (actual notes, normal notes and type). In MusicXML scores, tuplets elements can contain this information in <tuplet-actual> and <tuplet-normal> tags, but usually, they do not need to be specified and can be inferred from the <time-modification> tags of notes. Actually, MuseScore only exports such tags in case of nested tuplets as <time-modification> tags result from the multiplication of the different nested ratio.

Currently, Partitura does not parse those tags, and therefore some information is lost in case of nested tuplets. Even in the case of simple single-level tuplets, I feel that this information should be accessible at the Tuplet object level, in addition to the note level.

This PR allows exactly that, by adding optional actual_notes, normal_notes and type attributes, that are parsed from the MusicXML <tuplet-actual/normal> tags if present, and otherwise inferred from the first note of the tuplet.

This also adds a property duration_multiplier to the Tuplet similarly to what music21 proposes (which is simply normal_notes/actual_notes).

I also implemented it at export time, always outputting the tags <tuplet-actual/normal> no matter if they are absolutely needed or only optional, I don't think that it could cause any issue?

[manoskary]

Very nice addition to Partitura to cover better support of tuplets in musicxml . Review contains only minor comments, suggestions, and questions

[leleogere]

I case you have planned to merge this right now, do not yet, I've found an issue I would like some insights about. I'll explain in a comment on the related code in a few minutes.

[leleogere]

The issue I mentioned is about the following assert:
https://github.com/leleogere/partitura/blob/ab426ee43cb957b2a23ab17f8c5deb60a55b5fed/partitura/io/importmusicxml.py#L1453

assert tuplet_actual_type == tuplet_normal_type, "Tuplet types are not the same"

Initially, I thought that the <tuplet-type> would always be the same in <tuplet-actual> and <tuplet-normal>, as tuplets are usually 3 eighths in the space of 2 eighths, or 5 quarters in the space of 4 quarters... I couldn't really see a case where we would like to do like 3 eights in the space of 2 quarters, therefore, I assumed that this assert should be ok.

However, the tag belonging to <tuplet-actual> AND <tuplet-normal> should have been an insight that the MusicXML spec allows them to be different, and I actually found an example of such a piece:
https://github.com/CPJKU/asap-dataset/blob/main/Chopin/Ballades/1/xml_score.musicxml (lines 79875 to 79884)

          <tuplet number="1" placement="below" type="start">
            <tuplet-actual>
              <tuplet-number>13</tuplet-number>
              <tuplet-type>eighth</tuplet-type>
            </tuplet-actual>
            <tuplet-normal>
              <tuplet-number>4</tuplet-number>
              <tuplet-type>quarter</tuplet-type>
            </tuplet-normal>
          </tuplet>

This piece contains a lot of occurrences of tuplet_actual_type != tuplet_normal_type (simply search for tuplet-type in the xml), and therefore will trigger the AssertionError when parsed by Partitura.

The way MuseScore deals with it seems to be to convert the <tuplet-normal> unit type into the one in <tuplet-actual>. Therefore, in that case, it interprets the normal part as 8 eighth notes rather than 4 quarter notes as specified in the XML, resulting in a 13:8-eights tuplet:

Therefore here is my question, should I separate the type attribute of tuplets into two attributes normal_type and actual_type as specified in the xml? Or rather convert the normal-type in the same type as the actual_type as MuseScore does and keep a single type attribute as I've done now?

I would be interested in your insights about this issue @manoskary

[manoskary]

Hello Leo and thanks for the detailed message. It is true that we tend to find the weirdest things in score encodings and this would not be only example for sure. Generally, I would recommend using the option that would not break the parser. In this case I think separating the type could actually be a good solution for the import, in order to respect the encoding. For the export, I would use the MuseScore solution so it does not break. As always add some inline comments explaining why. Would this solution sound reasonable to you?

[leleogere]

I think separating the type might be the cleanest solution. However, if I separate the type, don't you think that I can simply export it as is? With the two different types as it was in the original xml?

[leleogere]

I've split the .type attribute into two .actual_type and .normal_type attribute. In the export, I've exported them as is, but let me know if you would rather do the conversion normal-type to actual-type anyway at export time. I had to deal with the conversion anyway in .duration_multiplier as it needs to be taken into account in case of actual_type != normal_type.

[manoskary]

Ok thanks let me run some tests before merging

[manoskary]

After running some tests on bigger Musicxml sets, loading and exporting is not failing therfore I will move forward with merging this PR. If in the future we notice, incorrect visualizations of tuplets, maybe we can rethink the export part.