From de901e6cbd7ffd4fb2161bd9bf05e4eadff9d529 Mon Sep 17 00:00:00 2001 From: Tim Beyer <35711942+TimFelixBeyer@users.noreply.github.com> Date: Sun, 20 Aug 2023 12:37:54 +0200 Subject: [PATCH 1/4] Fix music21 tag parsing No longer create hidden rests while parsing forward tags in musicXML files, as this led to problems. If there are unfilled gaps, they will be filled later by makeRests anyways if required. --- music21/musicxml/xmlToM21.py | 64 +++++------------------------------- 1 file changed, 9 insertions(+), 55 deletions(-) diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index c9d571185..13e65a752 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -31,6 +31,7 @@ from music21 import duration from music21 import dynamics from music21.common.enums import OrnamentDelay +from music21.common.numberTools import opFrac from music21 import editorial from music21 import environment from music21 import exceptions21 @@ -1760,32 +1761,8 @@ def parseMeasures(self): for mxMeasure in self.mxPart.iterfind('measure'): self.xmlMeasureToMeasure(mxMeasure) - self.removeEndForwardRest() part.coreElementsChanged() - def removeEndForwardRest(self): - ''' - If the last measure ended with a forward tag, as happens - in some pieces that end with incomplete measures, - and voices are not involved, - remove the rest there (for backwards compatibility, esp. - since bwv66.6 uses it) - - * New in v7. - ''' - if self.lastMeasureParser is None: # pragma: no cover - return # should not happen - lmp = self.lastMeasureParser - self.lastMeasureParser = None # clean memory - - if lmp.endedWithForwardTag is None: - return - if lmp.useVoices is True: - return - endedForwardRest = lmp.endedWithForwardTag - if lmp.stream.recurse().notesAndRests.last() is endedForwardRest: - lmp.stream.remove(endedForwardRest, recurse=True) - def separateOutPartStaves(self) -> list[stream.PartStaff]: ''' Take a `Part` with multiple staves and make them a set of `PartStaff` objects. @@ -2233,7 +2210,7 @@ def adjustTimeAttributesFromMeasure(self, m: stream.Measure): else: self.lastMeasureWasShort = False - self.lastMeasureOffset += mOffsetShift + self.lastMeasureOffset = opFrac(self.lastMeasureOffset + mOffsetShift) def applyMultiMeasureRest(self, r: note.Rest): ''' @@ -2390,13 +2367,6 @@ def __init__(self, # what is the offset in the measure of the current note position? self.offsetMeasureNote: OffsetQL = 0.0 - # keep track of the last rest that was added with a forward tag. - # there are many pieces that end with incomplete measures that - # older versions of Finale put a forward tag at the end, but this - # disguises the incomplete last measure. The PartParser will - # pick this up from the last measure. - self.endedWithForwardTag: note.Rest | None = None - @staticmethod def getStaffNumber(mxObjectOrNumber) -> int: ''' @@ -2596,10 +2566,8 @@ def xmlBackup(self, mxObj: ET.Element): ''' mxDuration = mxObj.find('duration') if durationText := strippedText(mxDuration): - change = common.numberTools.opFrac( - float(durationText) / self.divisions - ) - self.offsetMeasureNote -= change + change = opFrac(float(durationText) / self.divisions) + self.offsetMeasureNote = opFrac(self.offsetMeasureNote - change) # check for negative offsets produced by # musicxml durations with float rounding issues # https://github.com/cuthbertLab/music21/issues/971 @@ -2611,22 +2579,9 @@ def xmlForward(self, mxObj: ET.Element): ''' mxDuration = mxObj.find('duration') if durationText := strippedText(mxDuration): - change = common.numberTools.opFrac( - float(durationText) / self.divisions - ) - - # Create hidden rest (in other words, a spacer) - # old Finale documents close incomplete final measures with - # this will be removed afterward by removeEndForwardRest() - r = note.Rest(quarterLength=change) - r.style.hideObjectOnPrint = True - self.addToStaffReference(mxObj, r) - self.insertInMeasureOrVoice(mxObj, r) - + change = opFrac(float(durationText) / self.divisions) # Allow overfilled measures for now -- TODO(someday): warn? - self.offsetMeasureNote += change - # xmlToNote() sets None - self.endedWithForwardTag = r + self.offsetMeasureNote = opFrac(self.offsetMeasureNote + change) def xmlPrint(self, mxPrint: ET.Element): ''' @@ -2785,8 +2740,7 @@ def xmlToNote(self, mxNote: ET.Element) -> None: self.nLast = c # update # only increment Chords after completion - self.offsetMeasureNote += offsetIncrement - self.endedWithForwardTag = None + self.offsetMeasureNote = opFrac(self.offsetMeasureNote + offsetIncrement) def xmlToChord(self, mxNoteList: list[ET.Element]) -> chord.ChordBase: # noinspection PyShadowingNames @@ -3578,7 +3532,7 @@ def xmlToDuration(self, mxNote, inputM21=None): mxDuration = mxNote.find('duration') if mxDuration is not None: noteDivisions = float(mxDuration.text.strip()) - qLen = common.numberTools.opFrac(noteDivisions / divisions) + qLen = opFrac(noteDivisions / divisions) else: qLen = 0.0 @@ -5510,7 +5464,7 @@ def parseAttributesTag(self, mxAttributes): meth(mxSub) # NOT to be done: directive -- deprecated since v2. elif tag == 'divisions': - self.divisions = common.opFrac(float(mxSub.text)) + self.divisions = opFrac(float(mxSub.text)) # TODO: musicxml4: for-part including part-clef # TODO: instruments -- int if more than one instrument plays most of the time # TODO: part-symbol From b217557fa2845781fbe6cd8de30ffbc0c8abc48a Mon Sep 17 00:00:00 2001 From: Tim Beyer <35711942+TimFelixBeyer@users.noreply.github.com> Date: Sun, 20 Aug 2023 13:33:55 +0200 Subject: [PATCH 2/4] Remove incorrectly added rests Inspired by https://github.com/cuthbertLab/music21/issues/991 --- music21/musicxml/xmlToM21.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index 13e65a752..3319a7440 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -2524,7 +2524,7 @@ def parse(self): meth = getattr(self, methName) meth(mxObj) - if self.useVoices is True: + if self.useVoices: for v in self.stream.iter().voices: if v: # do not bother with empty voices # the musicDataMethods use insertCore, thus the voices need to run @@ -2536,6 +2536,21 @@ def parse(self): fillGaps=True, inPlace=True, hideRests=True) + # Remove rests incorrectly added to a staff where it's not required + # https://github.com/cuthbertLab/music21/issues/991 + for e in v.elements: + if e in elementsBefore: + continue + next_element = e.next() + for k, listOfEls in self.staffReference.items(): + if next_element in listOfEls: + staffKey = k + if next_element.offset < e.offset: + staffKey -= 1 + if staffKey >= 0: + self.staffReference.setdefault(staffKey, []).append(e) + break + self.stream.coreElementsChanged() if (self.restAndNoteCount['rest'] == 1 From 88871e6cf0ff9f29cf25b4abddd010d7247f0b23 Mon Sep 17 00:00:00 2001 From: Tim Beyer <35711942+TimFelixBeyer@users.noreply.github.com> Date: Sun, 20 Aug 2023 19:21:02 +0200 Subject: [PATCH 3/4] lint --- music21/musicxml/xmlToM21.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index 3319a7440..c67b3afea 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -2532,13 +2532,15 @@ def parse(self): v.coreElementsChanged() # Fill mid-measure gaps, and find end of measure gaps by ref to measure stream # https://github.com/cuthbertlab/music21/issues/444 + elementsBefore = v.elements + v.makeRests(refStreamOrTimeRange=self.stream, fillGaps=True, inPlace=True, hideRests=True) # Remove rests incorrectly added to a staff where it's not required # https://github.com/cuthbertLab/music21/issues/991 - for e in v.elements: + for e in v.elements: # pylint: disable=too-many-nested-blocks if e in elementsBefore: continue next_element = e.next() @@ -2550,7 +2552,6 @@ def parse(self): if staffKey >= 0: self.staffReference.setdefault(staffKey, []).append(e) break - self.stream.coreElementsChanged() if (self.restAndNoteCount['rest'] == 1 From 273e9b155d3972f362deeeb2dd4e5511a2e97d5c Mon Sep 17 00:00:00 2001 From: TimFelix <35711942+TimFelixBeyer@users.noreply.github.com> Date: Mon, 21 Aug 2023 02:43:28 +0200 Subject: [PATCH 4/4] Correctly deal with hidden rests and XML We simply wait until staves have been separated before creating rests. testHiddenRestImpliedVoice is modified since we no longer import the empty forward tag as a trailing duration. --- music21/_version.py | 2 +- music21/base.py | 2 +- music21/musicxml/test_xmlToM21.py | 2 +- music21/musicxml/xmlToM21.py | 46 +++++++++++-------------------- 4 files changed, 19 insertions(+), 33 deletions(-) diff --git a/music21/_version.py b/music21/_version.py index c5a4c6fd4..518c9a43a 100644 --- a/music21/_version.py +++ b/music21/_version.py @@ -47,7 +47,7 @@ ''' from __future__ import annotations -__version__ = '9.2.0b1' +__version__ = '9.2.0b2' def get_version_tuple(vv): v = vv.split('.') diff --git a/music21/base.py b/music21/base.py index f58f7f7af..c2a6e2b37 100644 --- a/music21/base.py +++ b/music21/base.py @@ -27,7 +27,7 @@ >>> music21.VERSION_STR -'9.2.0b1' +'9.2.0b2' Alternatively, after doing a complete import, these classes are available under the module "base": diff --git a/music21/musicxml/test_xmlToM21.py b/music21/musicxml/test_xmlToM21.py index 4847c1d16..10133b2b7 100644 --- a/music21/musicxml/test_xmlToM21.py +++ b/music21/musicxml/test_xmlToM21.py @@ -1290,7 +1290,7 @@ def testHiddenRestImpliedVoice(self): self.assertEqual(len(MP.stream.voices), 2) self.assertEqual(len(MP.stream.voices[0].elements), 1) - self.assertEqual(len(MP.stream.voices[1].elements), 2) + self.assertEqual(len(MP.stream.voices[1].elements), 1) self.assertEqual(MP.stream.voices[1].id, 'non-integer-value') def testMultiDigitEnding(self): diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index c67b3afea..b319db3e6 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -867,6 +867,19 @@ def xmlRootToScore(self, mxScore, inputM21=None): self.spannerBundle.remove(sp) s.coreElementsChanged() + # Fill gaps with rests where needed + for m in s[stream.Measure]: + for v in m.voices: + if v: # do not bother with empty voices + # the musicDataMethods use insertCore, thus the voices need to run + # coreElementsChanged + v.coreElementsChanged() + # Fill mid-measure gaps, and find end of measure gaps by ref to measure stream + # https://github.com/cuthbertlab/music21/issues/444 + v.makeRests(refStreamOrTimeRange=m, + fillGaps=True, + inPlace=True, + hideRests=True) s.definesExplicitSystemBreaks = self.definesExplicitSystemBreaks s.definesExplicitPageBreaks = self.definesExplicitPageBreaks for p in s.parts: @@ -2523,35 +2536,8 @@ def parse(self): if methName is not None: meth = getattr(self, methName) meth(mxObj) - - if self.useVoices: - for v in self.stream.iter().voices: - if v: # do not bother with empty voices - # the musicDataMethods use insertCore, thus the voices need to run - # coreElementsChanged - v.coreElementsChanged() - # Fill mid-measure gaps, and find end of measure gaps by ref to measure stream - # https://github.com/cuthbertlab/music21/issues/444 - elementsBefore = v.elements - - v.makeRests(refStreamOrTimeRange=self.stream, - fillGaps=True, - inPlace=True, - hideRests=True) - # Remove rests incorrectly added to a staff where it's not required - # https://github.com/cuthbertLab/music21/issues/991 - for e in v.elements: # pylint: disable=too-many-nested-blocks - if e in elementsBefore: - continue - next_element = e.next() - for k, listOfEls in self.staffReference.items(): - if next_element in listOfEls: - staffKey = k - if next_element.offset < e.offset: - staffKey -= 1 - if staffKey >= 0: - self.staffReference.setdefault(staffKey, []).append(e) - break + for v in self.stream[stream.Voice]: + v.coreElementsChanged() self.stream.coreElementsChanged() if (self.restAndNoteCount['rest'] == 1 @@ -2574,7 +2560,7 @@ def xmlBackup(self, mxObj: ET.Element): >>> mxBackup = EL('100') >>> MP.xmlBackup(mxBackup) >>> MP.offsetMeasureNote - 0.9979 + Fraction(9979, 10000) >>> MP.xmlBackup(mxBackup) >>> MP.offsetMeasureNote