From 3e9bfd0fd1226f2d6fcb0f8e521d007a82374ffb Mon Sep 17 00:00:00 2001 From: Greg Chapman <75333244+gregchapman-dev@users.noreply.github.com> Date: Thu, 15 May 2025 13:06:40 -0700 Subject: [PATCH 1/6] Proposed solution for PR #1636. Note that I had to check md['software'] as it was being constructed, instead of afterward, because Metadata() actually adds a software item for the music21 version that read the file, and I don't want to be confused by that. --- music21/_version.py | 2 +- music21/base.py | 2 +- music21/musicxml/xmlToM21.py | 28 +++++++++++++++++++--------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/music21/_version.py b/music21/_version.py index 1f44314e2..ced5aca10 100644 --- a/music21/_version.py +++ b/music21/_version.py @@ -50,7 +50,7 @@ ''' from __future__ import annotations -__version__ = '9.6.0b4' +__version__ = '9.6.0b19' def get_version_tuple(vv): v = vv.split('.') diff --git a/music21/base.py b/music21/base.py index 446b6d8e2..4044b96ac 100644 --- a/music21/base.py +++ b/music21/base.py @@ -27,7 +27,7 @@ >>> music21.VERSION_STR -'9.6.0b4' +'9.6.0b19' Alternatively, after doing a complete import, these classes are available under the module "base": diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index 5f635d797..dd996469f 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -769,6 +769,7 @@ def __init__(self): self.parts = [] self.musicXmlVersion = defaults.musicxmlVersion + self.wasWrittenByFinale = False def scoreFromFile(self, filename): ''' @@ -1326,9 +1327,17 @@ def processEncoding(self, encoding: ET.Element, md: metadata.Metadata) -> None: # TODO: encoder (text + type = role) multiple # TODO: encoding date multiple # TODO: encoding-description (string) multiple + finaleFound: bool = False + nonFinaleFound: bool = False for software in encoding.findall('software'): if softwareText := strippedText(software): + if 'Finale' in softwareText: + finaleFound = True + else: + nonFinaleFound = True md.add('software', softwareText) + if finaleFound and not nonFinaleFound: + self.wasWrittenByFinale = True for supports in encoding.findall('supports'): # todo: element: required @@ -2630,18 +2639,19 @@ def xmlForward(self, mxObj: ET.Element): if durationText := strippedText(mxDuration): change = 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) + if self.parent.parent.wasWrittenByFinale: + # 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) + # xmlToNote() sets None + self.endedWithForwardTag = r # Allow overfilled measures for now -- TODO(someday): warn? self.offsetMeasureNote += change - # xmlToNote() sets None - self.endedWithForwardTag = r def xmlPrint(self, mxPrint: ET.Element): ''' From 1a646211ec27d5d848cbf02d858771bb9b461417 Mon Sep 17 00:00:00 2001 From: Greg Chapman <75333244+gregchapman-dev@users.noreply.github.com> Date: Thu, 15 May 2025 13:48:06 -0700 Subject: [PATCH 2/6] Forgot about removing the makeRests call. --- music21/_version.py | 2 +- music21/base.py | 2 +- music21/musicxml/xmlToM21.py | 11 +---------- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/music21/_version.py b/music21/_version.py index ced5aca10..03c3aeb75 100644 --- a/music21/_version.py +++ b/music21/_version.py @@ -50,7 +50,7 @@ ''' from __future__ import annotations -__version__ = '9.6.0b19' +__version__ = '9.6.0b20' def get_version_tuple(vv): v = vv.split('.') diff --git a/music21/base.py b/music21/base.py index 4044b96ac..b4f62a650 100644 --- a/music21/base.py +++ b/music21/base.py @@ -27,7 +27,7 @@ >>> music21.VERSION_STR -'9.6.0b19' +'9.6.0b20' Alternatively, after doing a complete import, these classes are available under the module "base": diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index dd996469f..b9dc976c0 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -2584,16 +2584,7 @@ def parse(self): if self.useVoices is True: 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 - v.makeRests(refStreamOrTimeRange=self.stream, - fillGaps=True, - inPlace=True, - hideRests=True) + v.coreElementsChanged() self.stream.coreElementsChanged() if (self.restAndNoteCount['rest'] == 1 From 8b02acd270fa91fe50b14df40e0fdb8030e28caa Mon Sep 17 00:00:00 2001 From: Greg Chapman <75333244+gregchapman-dev@users.noreply.github.com> Date: Thu, 15 May 2025 14:11:09 -0700 Subject: [PATCH 3/6] Update version number and fix a few tests to no longer have hidden rests. --- music21/_version.py | 2 +- music21/base.py | 2 +- music21/musicxml/test_xmlToM21.py | 13 +++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/music21/_version.py b/music21/_version.py index 03c3aeb75..5e4bee74b 100644 --- a/music21/_version.py +++ b/music21/_version.py @@ -50,7 +50,7 @@ ''' from __future__ import annotations -__version__ = '9.6.0b20' +__version__ = '9.6.0b21' def get_version_tuple(vv): v = vv.split('.') diff --git a/music21/base.py b/music21/base.py index b4f62a650..b91b3b783 100644 --- a/music21/base.py +++ b/music21/base.py @@ -27,7 +27,7 @@ >>> music21.VERSION_STR -'9.6.0b20' +'9.6.0b21' 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 005172424..4b6fb6560 100644 --- a/music21/musicxml/test_xmlToM21.py +++ b/music21/musicxml/test_xmlToM21.py @@ -1328,12 +1328,13 @@ def testHiddenRests(self): # Voice 2: (half), quarter note, (quarter) s = converter.parse(testPrimitive.hiddenRests) v1, v2 = s.recurse().voices - self.assertEqual(v1.duration.quarterLength, v2.duration.quarterLength) + self.assertEqual(v1.duration.quarterLength, 4.0) + self.assertEqual(v2.duration.quarterLength, 3.0) - restV1 = v1.getElementsByClass(note.Rest)[0] - self.assertTrue(restV1.style.hideObjectOnPrint) - restsV2 = v2.getElementsByClass(note.Rest) - self.assertEqual([r.style.hideObjectOnPrint for r in restsV2], [True, True]) + restsV1 = list(v1.getElementsByClass(note.Rest)) + self.assertEqual(restsV1, []) + restsV2 = list(v2.getElementsByClass(note.Rest)) + self.assertEqual(restsV2, []) # Schoenberg op.19/2 # previously, last measure of LH duplicated hidden rest belonging to RH @@ -1367,7 +1368,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): From acbf2733272f4bba1f2c5f61b8c4ba556a041df3 Mon Sep 17 00:00:00 2001 From: Greg Chapman <75333244+gregchapman-dev@users.noreply.github.com> Date: Sat, 17 May 2025 12:06:21 -0700 Subject: [PATCH 4/6] Don't call v.coreElementsChanged on empty voices. --- music21/_version.py | 2 +- music21/base.py | 2 +- music21/musicxml/xmlToM21.py | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/music21/_version.py b/music21/_version.py index 5e4bee74b..31df28d10 100644 --- a/music21/_version.py +++ b/music21/_version.py @@ -50,7 +50,7 @@ ''' from __future__ import annotations -__version__ = '9.6.0b21' +__version__ = '9.6.0b24' def get_version_tuple(vv): v = vv.split('.') diff --git a/music21/base.py b/music21/base.py index b91b3b783..386b99761 100644 --- a/music21/base.py +++ b/music21/base.py @@ -27,7 +27,7 @@ >>> music21.VERSION_STR -'9.6.0b21' +'9.6.0b24' Alternatively, after doing a complete import, these classes are available under the module "base": diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index b9dc976c0..da3b8bc11 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -2584,7 +2584,10 @@ def parse(self): if self.useVoices is True: for v in self.stream.iter().voices: - v.coreElementsChanged() + if v: # do not bother with empty voices + # the musicDataMethods use insertCore, thus the voices need to run + # coreElementsChanged + v.coreElementsChanged() self.stream.coreElementsChanged() if (self.restAndNoteCount['rest'] == 1 From c6a6870e0311ad5da19329049e7e75b22d593b55 Mon Sep 17 00:00:00 2001 From: Greg Chapman <75333244+gregchapman-dev@users.noreply.github.com> Date: Sat, 17 May 2025 12:17:11 -0700 Subject: [PATCH 5/6] Update to include Finale and non-Finale tests from PR @1636. --- music21/musicxml/testPrimitive.py | 88 ++++++++++++++++++++++++++++--- music21/musicxml/test_xmlToM21.py | 23 +++++--- 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/music21/musicxml/testPrimitive.py b/music21/musicxml/testPrimitive.py index a37258c64..bee3baf4f 100644 --- a/music21/musicxml/testPrimitive.py +++ b/music21/musicxml/testPrimitive.py @@ -18823,7 +18823,83 @@ ''' -hiddenRests = ''' +hiddenRestsFinale = ''' + + + + + Finale 2014 for Mac + + + + + MusicXML Part + + + + + + 2 + + + G + 2 + + + + + E + 5 + + 4 + 1 + half + up + + + 2 + 1 + + + + E + 4 + + 2 + 1 + quarter + up + + + 8 + + + 4 + 2 + + + + F + 4 + + 2 + 2 + quarter + down + + + 2 + 2 + + + + +''' + +hiddenRestsNoFinale = ''' @@ -18946,7 +19022,6 @@ ''' - tupletsImplied = ''' @@ -20600,10 +20675,11 @@ mixedVoices1a, mixedVoices1b, mixedVoices2, # 37 colors01, triplets01, textBoxes01, octaveShifts33d, # 40 unicodeStrNoNonAscii, unicodeStrWithNonAscii, # 44 - tremoloTest, hiddenRests, multiDigitEnding, tupletsImplied, pianoStaffPolymeter, # 46 - arpeggio32d, multiStaffArpeggios, multiMeasureEnding, # 51 - pianoStaffPolymeterWithClefOctaveChange, multipleFingeringsOnChord, # 54 - pianoStaffWithOttava, pedalLines, pedalSymLines # 56 + tremoloTest, hiddenRestsFinale, hiddenRestsNoFinale, multiDigitEnding, # 46 + tupletsImplied, pianoStaffPolymeter, arpeggio32d, multiStaffArpeggios, # 50 + multiMeasureEnding, pianoStaffPolymeterWithClefOctaveChange, # 54 + multipleFingeringsOnChord, pianoStaffWithOttava, # 56 + pedalLines, pedalSymLines # 58 ] diff --git a/music21/musicxml/test_xmlToM21.py b/music21/musicxml/test_xmlToM21.py index 4b6fb6560..d333e48c2 100644 --- a/music21/musicxml/test_xmlToM21.py +++ b/music21/musicxml/test_xmlToM21.py @@ -1326,15 +1326,24 @@ def testHiddenRests(self): # Voice 1: Half note, (quarter), quarter note # Voice 2: (half), quarter note, (quarter) - s = converter.parse(testPrimitive.hiddenRests) + s = converter.parse(testPrimitive.hiddenRestsNoFinale) v1, v2 = s.recurse().voices - self.assertEqual(v1.duration.quarterLength, 4.0) - self.assertEqual(v2.duration.quarterLength, 3.0) + # No rests should have been added + self.assertFalse(v1.getElementsByClass(note.Rest)) + self.assertFalse(v2.getElementsByClass(note.Rest)) - restsV1 = list(v1.getElementsByClass(note.Rest)) - self.assertEqual(restsV1, []) - restsV2 = list(v2.getElementsByClass(note.Rest)) - self.assertEqual(restsV2, []) + # Finale uses tags to represent hidden rests, + # so we want to have rests here + # Voice 1: Half note, (quarter), quarter note + # Voice 2: (half), quarter note, (quarter) + s = converter.parse(testPrimitive.hiddenRestsFinale) + v1, v2 = s.recurse().voices + self.assertEqual(v1.duration.quarterLength, v2.duration.quarterLength) + + restV1 = v1.getElementsByClass(note.Rest)[0] + self.assertTrue(restV1.style.hideObjectOnPrint) + restsV2 = v2.getElementsByClass(note.Rest) + self.assertEqual([r.style.hideObjectOnPrint for r in restsV2], [True, True]) # Schoenberg op.19/2 # previously, last measure of LH duplicated hidden rest belonging to RH From c6b7607b5dc8175b93afbda4c8ce34b2daebf1b3 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Wed, 21 May 2025 23:22:03 -1000 Subject: [PATCH 6/6] rename for Myke to understand, add safety Add safety for MeasureParsers created w/o Part/ScoreParsers (testing, extensions, etc.) Add TESTS to make sure that Finale workarounds are only done if the score was last exported by Finale. Tests are mostly on processing Encoding, which suddenly matters a lot! :-) --- music21/musicxml/test_xmlToM21.py | 60 +++++++++++++++++++-- music21/musicxml/xmlToM21.py | 88 +++++++++++++++++++------------ 2 files changed, 109 insertions(+), 39 deletions(-) diff --git a/music21/musicxml/test_xmlToM21.py b/music21/musicxml/test_xmlToM21.py index d333e48c2..6f71f495d 100644 --- a/music21/musicxml/test_xmlToM21.py +++ b/music21/musicxml/test_xmlToM21.py @@ -17,6 +17,7 @@ from music21 import instrument from music21 import key from music21 import layout +from music21 import metadata from music21 import meter from music21 import note from music21 import pitch @@ -33,11 +34,6 @@ ) class Test(unittest.TestCase): - def testParseSimple(self): - MI = MusicXMLImporter() - MI.xmlText = r'''''' - self.assertRaises(MusicXMLImportException, MI.parseXMLText) - def EL(self, elText): return ET.fromstring(elText) @@ -53,6 +49,60 @@ def pitchOut(self, listIn): out += ']' return out + def testParseSimple(self): + MI = MusicXMLImporter() + MI.xmlText = r'''''' + self.assertRaises(MusicXMLImportException, MI.parseXMLText) + + def test_processEncoding(self): + ''' + Test that the Encoding tag sets software etc. properly. + ''' + enc1 = ''' + + 2025-05-21 + Finale v26.3 for Mac + + + + ''' + mxl_importer = MusicXMLImporter() + self.assertFalse(mxl_importer.applyFinaleWorkarounds) + self.assertFalse(mxl_importer.definesExplicitSystemBreaks) + self.assertFalse(mxl_importer.definesExplicitPageBreaks) + + encoding = self.EL(enc1) + md = metadata.Metadata() + self.assertEqual(len(md.software), 1) + # we add music21 to all initial software... + self.assertIn('music21', md.software[0]) + + mxl_importer = MusicXMLImporter() + mxl_importer.processEncoding(encoding, md) + self.assertTrue(mxl_importer.applyFinaleWorkarounds) + self.assertTrue(mxl_importer.definesExplicitSystemBreaks) + self.assertTrue(mxl_importer.definesExplicitPageBreaks) + self.assertIn('Finale v26.3 for Mac', md.software) + + enc1 = ''' + + 2099-05-21 + music21 v.99 + Finale v90 for ChatGPT Implant + + + + ''' + mxl_importer = MusicXMLImporter() + encoding = self.EL(enc1) + md = metadata.Metadata() + mxl_importer.processEncoding(encoding, md) + self.assertFalse(mxl_importer.applyFinaleWorkarounds) + self.assertFalse(mxl_importer.definesExplicitSystemBreaks) + self.assertTrue(mxl_importer.definesExplicitPageBreaks) + self.assertIn('music21 v.99', md.software) + self.assertIn('Finale v90 for ChatGPT Implant', md.software) + def testExceptionMessage(self): mxScorePart = self.EL('Elec.') mxPart = self.EL('thirty-tooth') diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index da3b8bc11..862048637 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -769,7 +769,11 @@ def __init__(self): self.parts = [] self.musicXmlVersion = defaults.musicxmlVersion - self.wasWrittenByFinale = False + + # Finale (RIP 2025) had a problem with writing extraneous tags. + # if this is True then we will be cautious before interpreting them as + # hidden rests. + self.applyFinaleWorkarounds = False def scoreFromFile(self, filename): ''' @@ -1325,19 +1329,22 @@ def processEncoding(self, encoding: ET.Element, md: metadata.Metadata) -> None: * new-page = Metadata.definesExplicitPageBreaks ''' # TODO: encoder (text + type = role) multiple - # TODO: encoding date multiple + # TODO: encoding-date either singular or multiple # TODO: encoding-description (string) multiple - finaleFound: bool = False - nonFinaleFound: bool = False + + # If the first software tag contains Finale, then it + # is by finale. Otherwise, it is not + foundOneSoftwareTag: bool = False + finaleIsFirst: bool = False for software in encoding.findall('software'): if softwareText := strippedText(software): - if 'Finale' in softwareText: - finaleFound = True - else: - nonFinaleFound = True + if not foundOneSoftwareTag: + if 'Finale' in softwareText: + finaleIsFirst = True + foundOneSoftwareTag = True md.add('software', softwareText) - if finaleFound and not nonFinaleFound: - self.wasWrittenByFinale = True + if finaleIsFirst: + self.applyFinaleWorkarounds = True for supports in encoding.findall('supports'): # todo: element: required @@ -1769,16 +1776,20 @@ def parseMeasures(self): for mxMeasure in self.mxPart.iterfind('measure'): self.xmlMeasureToMeasure(mxMeasure) - self.removeEndForwardRest() + self.removeFinaleIncorrectEndingForwardRest() part.coreElementsChanged() - def removeEndForwardRest(self): + def removeFinaleIncorrectEndingForwardRest(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) + If Finale generated the file AND it ended with an incomplete + measure (like 4/4 beginning with a quarter pickup and ending + with a 3-beat measure) then the file might have ended with a + `` tag, which Finale used to create hidden rests. + + If this forward tag is at the end of the piece, then it + will create rests that "complete" the measure in an incorrect way + If voices are not involved (e.g., NOT bwv66.6) then we should + remove this forward tag. * New in v7. ''' @@ -1787,13 +1798,14 @@ def removeEndForwardRest(self): lmp = self.lastMeasureParser self.lastMeasureParser = None # clean memory - if lmp.endedWithForwardTag is None: + if lmp.lastForwardTagCreatedByFinale 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) + endingForwardRest: note.Rest|None = lmp.lastForwardTagCreatedByFinale + # important that we find that the last GeneralNote is this Forward tag + if lmp.stream[note.GeneralNote].last() is endingForwardRest: + lmp.stream.remove(endingForwardRest, recurse=True) def separateOutPartStaves(self) -> list[stream.PartStaff]: ''' @@ -2412,12 +2424,16 @@ 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 + # Keep track of the last rest that was added with a forward tag. + + # Older versions of Finale put a tag at the end of pieces + # which ended with an incomplete measure. Find that last + # Forward tag (if created by Finale) and store it. + # if later we find that this measure is the last one, + # and doesn't have multiple voices, and was created by Finale, + # then we'll delete the Rest associated with this forward tag + # at the cleanup stage of PartParser. + self.lastForwardTagCreatedByFinale: note.Rest|None = None # Temporary storage of intended start offset of a PedalMark (we sometimes # need to know this before the PedalMark or its first element have been @@ -2633,16 +2649,20 @@ def xmlForward(self, mxObj: ET.Element): if durationText := strippedText(mxDuration): change = opFrac(float(durationText) / self.divisions) - if self.parent.parent.wasWrittenByFinale: - # Create hidden rest (in other words, a spacer) - # old Finale documents close incomplete final measures with - # this will be removed afterward by removeEndForwardRest() + if (self.parent + and self.parent.parent + and self.parent.parent.applyFinaleWorkarounds): + # If the ScoreParser senses the Score was written by Finale + # then Forward tags need to create hidden rests (except + # at the end of the piece!) So create a hidden rest (spacer) here. r = note.Rest(quarterLength=change) r.style.hideObjectOnPrint = True self.addToStaffReference(mxObj, r) self.insertInMeasureOrVoice(mxObj, r) - # xmlToNote() sets None - self.endedWithForwardTag = r + + # old Finale documents close incomplete final measures with + # this will be removed afterward by removeFinaleIncorrectEndingForwardRest() + self.lastForwardTagCreatedByFinale = r # Allow overfilled measures for now -- TODO(someday): warn? self.offsetMeasureNote += change @@ -2805,7 +2825,7 @@ def xmlToNote(self, mxNote: ET.Element) -> None: # only increment Chords after completion self.offsetMeasureNote += offsetIncrement - self.endedWithForwardTag = None + self.lastForwardTagCreatedByFinale = None def xmlToChord(self, mxNoteList: list[ET.Element]) -> chord.ChordBase: # noinspection PyShadowingNames