From 0966cb3ad8c97f08b6e92e517414936c04e28f5d Mon Sep 17 00:00:00 2001 From: Felippe Roza <4269853+FelippeRoza@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:08:32 +0200 Subject: [PATCH] Fix bar counting and markup placement in MusicXML export The xfail tests test_partial and test_markup both broke at 5d89981. Bars close on accumulated note duration rather than at barchecks, and several things were not counted right: * set_pickup() discarded the \partial duration, so the pickup bar never closed and swallowed the measure after it. Its length is now recorded and used, and the bar is exported as . * Note-attached markup is parsed after its note, so it landed one measure late and texts from different bars were concatenated into one . Whether a markup trails its note is now taken from its Postfix parent instead of the current bar's contents. * Chord notes after the first were added to whichever bar was current by then, leaving an orphaned opening the next measure. * Grace notes were counted toward bar duration, so one could close a bar on its own and strand itself with no principal note to ornament. * \partial and bar duration leaked from one part into the next. The last three happen without \partial too. chord_duration.xml is regenerated because its expectation held the split chord it was added to catch in #171. Fixes #174 --- ly/musicxml/create_musicxml.py | 5 +- ly/musicxml/ly2xml_mediator.py | 51 ++++++++--- ly/musicxml/lymus2musxml.py | 12 ++- tests/test_xml.py | 11 ++- tests/test_xml_files/chord_duration.xml | 6 +- tests/test_xml_files/incomplete_bar.ly | 8 ++ tests/test_xml_files/incomplete_bar.xml | 115 ++++++++++++++++++++++++ tests/test_xml_files/markup.xml | 2 +- tests/test_xml_files/markup_mark.ly | 13 +++ tests/test_xml_files/markup_mark.xml | 76 ++++++++++++++++ tests/test_xml_files/partial.xml | 4 +- 11 files changed, 279 insertions(+), 24 deletions(-) create mode 100644 tests/test_xml_files/incomplete_bar.ly create mode 100644 tests/test_xml_files/incomplete_bar.xml create mode 100644 tests/test_xml_files/markup_mark.ly create mode 100644 tests/test_xml_files/markup_mark.xml diff --git a/ly/musicxml/create_musicxml.py b/ly/musicxml/create_musicxml.py index 6d62fb0..29ad683 100644 --- a/ly/musicxml/create_musicxml.py +++ b/ly/musicxml/create_musicxml.py @@ -122,7 +122,10 @@ def create_measure(self, pickup = False, **bar_attrs): """Create new measure """ if pickup and self.bar_nr == 1: self.bar_nr = 0 - self.current_bar = etree.SubElement(self.current_part, "measure", number=str(self.bar_nr)) + attrs = {'number': str(self.bar_nr)} + if self.bar_nr == 0: + attrs['implicit'] = 'yes' + self.current_bar = etree.SubElement(self.current_part, "measure", **attrs) self.bar_nr +=1 if bar_attrs: self.new_bar_attr(**bar_attrs) diff --git a/ly/musicxml/ly2xml_mediator.py b/ly/musicxml/ly2xml_mediator.py index e8c7d72..9132f59 100644 --- a/ly/musicxml/ly2xml_mediator.py +++ b/ly/musicxml/ly2xml_mediator.py @@ -61,6 +61,7 @@ def __init__(self): self.group = None self.group_num = 0 self.current_chord = [] + self.chord_bar = None self.q_chord = [] self.prev_pitch = None self.prev_chord_pitch = None @@ -83,6 +84,8 @@ def __init__(self): self.multiple_rest_bar = None self.current_mark = 1 self.bar_is_pickup = False + self.pickup_dura = None + self.in_grace = False self.stem_dir = None def new_header_assignment(self, name, value): @@ -160,6 +163,7 @@ def new_part(self, pid=None, to_part=None, piano=False): self.score.partlist.append(self.part) self.insert_into = self.part self.bar = None + self.reset_bar_progress() def part_not_empty(self): return self.part and self.part.barlist @@ -318,8 +322,11 @@ def get_first_var(self): if self.sections: return self.sections[0].barlist - def set_pickup(self): + def set_pickup(self, token, tokens): + r"""Set the length of the incomplete bar declared by \partial.""" self.bar_is_pickup = True + # A zero-length \partial declares no pickup bar at all. + self.pickup_dura = ly.duration.fraction((token,) + tokens) or None def new_bar(self, fill_prev=True): if self.bar and fill_prev: @@ -388,15 +395,21 @@ def new_mark(self, num_mark = None): self.current_attr.set_mark(self.bijective(self.current_mark)) self.current_mark += 1 - def new_word(self, word): + def new_word(self, word, trails_note=False): if self.bar is None: self.new_bar() - if self.bar.has_attr(): - self.current_attr.set_word(word) - else: - new_bar_attr = xml_objs.BarAttr() - new_bar_attr.set_word(word) - self.add_to_bar(new_bar_attr) + bar = self.bar + barlist = self.insert_into.barlist + if (trails_note and not bar.has_music() + and len(barlist) > 1 and bar is barlist[-1]): + # The note this markup trails completed the previous bar. + bar = barlist[-2] + attr = next((obj for obj in bar.obj_list + if isinstance(obj, xml_objs.BarAttr)), None) + if attr is None: + attr = xml_objs.BarAttr() + bar.add(attr) + attr.set_word(word) def new_time(self, num, den, numeric=False): self.current_time = Fraction(num, den.denominator) @@ -421,10 +434,23 @@ def new_clef(self, clefname): def set_relative(self, note): self.prev_pitch = note.pitch + def reset_bar_progress(self): + """Forget the bar position, which means nothing in the next part.""" + self.bar_dura = Fraction(0, 4) + self.pickup_dura = None + self.bar_is_pickup = False + + def set_grace_seq(self, value): + self.in_grace = value + def increase_bar_dura(self, duration): + if self.in_grace: + # Grace notes ornament the next note; they take up no bar time. + return self.bar_dura += duration[0] * duration[1] - if self.bar_dura >= self.current_time: - self.bar_dura = 0 + bar_length = self.current_time if self.pickup_dura is None else self.pickup_dura + if self.bar_dura >= bar_length: + self.reset_bar_progress() self.new_bar() def new_note(self, note, rel=False, is_unpitched=False): @@ -565,6 +591,8 @@ def new_chordbase(self, note, duration, rel=False): self.current_note.set_duration(duration) self.current_lynote = note self.check_current_note(rel) + # The other chord notes follow, possibly after this bar was closed. + self.chord_bar = self.bar self.increase_bar_dura(duration) def new_chordnote(self, note, rel): @@ -582,7 +610,7 @@ def new_chordnote(self, note, rel): chord_note.set_octave(p.octave + 3) self.prev_chord_pitch = p chord_note.chord = True - self.bar.add(chord_note) + self.chord_bar.add(chord_note) return chord_note def copy_prev_chord(self, duration): @@ -607,6 +635,7 @@ def copy_prev_chord(self, duration): def clear_chord(self): self.q_chord = self.current_chord self.current_chord = [] + self.chord_bar = None self.prev_chord_pitch = None def chord_end(self): diff --git a/ly/musicxml/lymus2musxml.py b/ly/musicxml/lymus2musxml.py index 11ed001..2e9b63b 100644 --- a/ly/musicxml/lymus2musxml.py +++ b/ly/musicxml/lymus2musxml.py @@ -91,6 +91,7 @@ def __init__(self): self.phrslurnr = 0 self.mark = False self.pickup = False + self.markup_trails_note = False def parse_text(self, ly_text, filename=None): """Parse the LilyPond source specified as text. @@ -346,7 +347,7 @@ def Duration(self, duration): self.mediator.set_tuplspan_dur(duration.token, duration.tokens) self.tupl_span = False elif self.pickup: - self.mediator.set_pickup() + self.mediator.set_pickup(duration.token, duration.tokens) self.pickup = False else: self.mediator.new_duration_token(duration.token, duration.tokens) @@ -440,6 +441,7 @@ def Dynamic(self, dynamic): def Grace(self, grace): self.grace_seq = True + self.mediator.set_grace_seq(True) def TimeSignature(self, timeSign): self.mediator.new_time(timeSign.numerator(), timeSign.fraction(), self.numericTime) @@ -529,10 +531,13 @@ def UserCommand(self, usercommand): self.tupl_span = True def Markup(self, markup): - pass + # Only markup written as a postfix event (-, ^ or _) follows its note; + # \mark, \tempo and standalone markup precede it. + self.markup_trails_note = isinstance(markup.parent(), + ly.music.items.Postfix) def MarkupWord(self, markupWord): - self.mediator.new_word(markupWord.token) + self.mediator.new_word(markupWord.token, self.markup_trails_note) def MarkupList(self, markuplist): pass @@ -622,6 +627,7 @@ def End(self, end): self.fraction = None elif isinstance(end.node, ly.music.items.Grace): #Grace self.grace_seq = False + self.mediator.set_grace_seq(False) elif end.node.token == '\\repeat': if end.node.specifier() == 'volta': self.mediator.new_repeat('backward') diff --git a/tests/test_xml.py b/tests/test_xml.py index e5ca26b..2bd7c86 100644 --- a/tests/test_xml.py +++ b/tests/test_xml.py @@ -7,7 +7,6 @@ import os import os.path import io -import pytest import re import sys @@ -46,7 +45,6 @@ def test_mark(): compare_output('mark') -@pytest.mark.xfail def test_partial(): compare_output('partial') @@ -63,11 +61,18 @@ def test_church(): compare_output('church_modes') -@pytest.mark.xfail def test_markup(): compare_output('markup') +def test_markup_mark(): + compare_output('markup_mark') + + +def test_incomplete_bar(): + compare_output('incomplete_bar') + + def test_breathe(): compare_output('breathe') diff --git a/tests/test_xml_files/chord_duration.xml b/tests/test_xml_files/chord_duration.xml index d47b1fc..cdf98fb 100644 --- a/tests/test_xml_files/chord_duration.xml +++ b/tests/test_xml_files/chord_duration.xml @@ -5,7 +5,7 @@ python-ly 0.9.10 - 2026-01-26 + 2026-08-17 @@ -52,8 +52,6 @@ 1 whole - - @@ -74,6 +72,8 @@ 1 whole + + E diff --git a/tests/test_xml_files/incomplete_bar.ly b/tests/test_xml_files/incomplete_bar.ly new file mode 100644 index 0000000..078ad03 --- /dev/null +++ b/tests/test_xml_files/incomplete_bar.ly @@ -0,0 +1,8 @@ +\version "2.18.2" + +\score { + << + \new Staff \relative { c''4 d4 } + \new Staff \relative { c'4 d4 e4 f4 | g1 } + >> +} diff --git a/tests/test_xml_files/incomplete_bar.xml b/tests/test_xml_files/incomplete_bar.xml new file mode 100644 index 0000000..42f1c98 --- /dev/null +++ b/tests/test_xml_files/incomplete_bar.xml @@ -0,0 +1,115 @@ + + + + + + python-ly 0.9.10 + 2026-08-17 + + + + + + + + + + + + + + 1 + + + G + 2 + + + + + C + 6 + + 1 + 1 + quarter + + + + D + 6 + + 1 + 1 + quarter + + + + + + + + 1 + + + G + 2 + + + + + C + 5 + + 1 + 1 + quarter + + + + D + 5 + + 1 + 1 + quarter + + + + E + 5 + + 1 + 1 + quarter + + + + F + 5 + + 1 + 1 + quarter + + + + + + G + 5 + + 4 + 1 + whole + + + + diff --git a/tests/test_xml_files/markup.xml b/tests/test_xml_files/markup.xml index 24db966..509b8f9 100644 --- a/tests/test_xml_files/markup.xml +++ b/tests/test_xml_files/markup.xml @@ -1,4 +1,4 @@ - + diff --git a/tests/test_xml_files/markup_mark.ly b/tests/test_xml_files/markup_mark.ly new file mode 100644 index 0000000..6d1ad55 --- /dev/null +++ b/tests/test_xml_files/markup_mark.ly @@ -0,0 +1,13 @@ +\version "2.18.2" + +% Markup not attached to a note (here \mark) belongs to the bar it is written in +% Note: emitting both and for one \mark is a known defect (#121) + +\score { + \relative { + a'1^\markup intenso | + \mark \markup Coda + a1 | + a1 + } +} diff --git a/tests/test_xml_files/markup_mark.xml b/tests/test_xml_files/markup_mark.xml new file mode 100644 index 0000000..a5caba7 --- /dev/null +++ b/tests/test_xml_files/markup_mark.xml @@ -0,0 +1,76 @@ + + + + + + python-ly 0.9.10 + 2026-08-17 + + + + + + + + + + + 1 + + + G + 2 + + + + + intenso + + + + + A + 4 + + 4 + 1 + whole + + + + + + + A + + + Coda + + + + + A + 4 + + 4 + 1 + whole + + + + + + A + 4 + + 4 + 1 + whole + + + + diff --git a/tests/test_xml_files/partial.xml b/tests/test_xml_files/partial.xml index cf1ef54..487f8e0 100644 --- a/tests/test_xml_files/partial.xml +++ b/tests/test_xml_files/partial.xml @@ -1,4 +1,4 @@ - + @@ -14,7 +14,7 @@ - + 2