Interpret empty <forward> tags as rests on Finale documents only
#1636
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -867,6 +867,21 @@ def xmlRootToScore(self, mxScore, inputM21=None): | |
| self.spannerBundle.remove(sp) | ||
|
|
||
| s.coreElementsChanged() | ||
| # Fill gaps with rests where needed | ||
| for m in s[stream.Measure]: | ||
|
Member
There was a problem hiding this comment. Why move this out of the MeasureParser? If it's just to have access to the
Contributor
Author
There was a problem hiding this comment. I had a look - I moved it there since it didn't play nice with partwise musicxml files. I think this is because the rests inserted by One example is schoenberg op19.2. -
Contributor
Author
There was a problem hiding this comment. I wasn't able to quickly resolve this in another way - lmk if you have any thoughts.
Contributor
Author
There was a problem hiding this comment. I think this test wasn't failing before because we got lucky and this particular case happened in the last measure of the piece, where the incorrect rest was removed by
Member
There was a problem hiding this comment. Thanks for the ping. I wanted to see what you were talking about, so here's what I stubbed out: diff --git a/music21/musicxml/test_xmlToM21.py b/music21/musicxml/test_xmlToM21.py
index ab1e99879..c8fe4421c 100644
--- a/music21/musicxml/test_xmlToM21.py
+++ b/music21/musicxml/test_xmlToM21.py
@@ -1547,4 +1547,4 @@ class Test(unittest.TestCase):
if __name__ == '__main__':
import music21
- music21.mainTest(Test)
+ music21.mainTest('noDocTest', Test, runTest="testHiddenRests")
diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py
index 0caa04f55..b385aeaab 100644
--- a/music21/musicxml/xmlToM21.py
+++ b/music21/musicxml/xmlToM21.py
@@ -821,7 +821,7 @@ class MusicXMLImporter(XMLParserBase):
self.musicXmlVersion = mxVersion
md = self.xmlMetadata(mxScore)
- s.coreInsert(0, md)
+ s.insert(0, md)
mxDefaults = mxScore.find('defaults')
if mxDefaults is not None:
@@ -867,21 +867,7 @@ class MusicXMLImporter(XMLParserBase):
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
- # but only when the score comes from Finale
- if any('Finale' in software for software in md.software):
- v.makeRests(refStreamOrTimeRange=m,
- fillGaps=True,
- inPlace=True,
- hideRests=True)
+
s.definesExplicitSystemBreaks = self.definesExplicitSystemBreaks
s.definesExplicitPageBreaks = self.definesExplicitPageBreaks
for p in s.parts:
@@ -2558,8 +2544,25 @@ class MeasureParser(SoundTagMixin, XMLParserBase):
if methName is not None:
meth = getattr(self, methName)
meth(mxObj)
+ md = self.parent.parent.stream[metadata.Metadata].first()
for v in self.stream[stream.Voice]:
- v.coreElementsChanged()
+ 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
+ # but only when the score comes from Finale
+ if md and any('Finale' in software for software in md.software):
+ old_ids = {id(el) for el in v}
+ v.makeRests(refStreamOrTimeRange=self.stream,
+ fillGaps=True,
+ inPlace=True,
+ hideRests=True)
+ for el in v:
+ if id(el) not in old_ids:
+ self.addToStaffReference(self.mxMeasure, el)
+
self.stream.coreElementsChanged()
if (self.restAndNoteCount['rest'] == 1Now I get: File "/Users/jwalls/music21/music21/musicxml/test_xmlToM21.py", line 1282, in testHiddenRests
self.assertIsInstance(hiddenRest, note.Rest)
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 1294, in assertIsInstance
self.fail(self._formatMessage(msg, standardMsg))
File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 715, in fail
raise self.failureException(msg)
AssertionError: <music21.chord.Chord E-5 F#5 B-5 D6> is not an instance of <class 'music21.note.Rest'>
Thanks for your persistence.
Member
There was a problem hiding this comment.
Do you mean MusicXML files where some parts have more than one staff? Almost all MusicXML files are Partwise, and I don't think music21 even imports Timewise since I've never seen one in the wild.
Contributor
Author
There was a problem hiding this comment. Yeah I meant multi-staff parts, sorry. Basically anything with a
Contributor
Author
There was a problem hiding this comment. @jacobtylerwalls i'm not 100% how
Member
There was a problem hiding this comment.
Forget that, I must have not used @TimFelixBeyer to your question, you're absolutely right that we need the staff reference. The bulletproof way to do that would be to get it from the |
||
| 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() | ||
|
Member
There was a problem hiding this comment. Note to self: if we decide to leave this here after all, this is probably not needed (but still think MeasureParser is better if I'm not missing something)
Member
There was a problem hiding this comment. It's been a while since I've thought through this PR, but I think that this is correct. |
||
| # Fill mid-measure gaps, and find end of measure gaps by ref to measure stream | ||
| # https://github.com/cuthbertlab/music21/issues/444 | ||
| # but only when the score comes from Finale | ||
| if any('Finale' in software for software in md.software): | ||
| v.makeRests(refStreamOrTimeRange=m, | ||
| fillGaps=True, | ||
| inPlace=True, | ||
| hideRests=True) | ||
| s.definesExplicitSystemBreaks = self.definesExplicitSystemBreaks | ||
| s.definesExplicitPageBreaks = self.definesExplicitPageBreaks | ||
| for p in s.parts: | ||
|
|
@@ -1761,32 +1776,8 @@ def parseMeasures(self): | |
| for mxMeasure in self.mxPart.iterfind('measure'): | ||
| self.xmlMeasureToMeasure(mxMeasure) | ||
|
|
||
| part.coreElementsChanged() | ||
|
|
||
|
Member
There was a problem hiding this comment. I think we can remove this instance attribute now. |
||
| def separateOutPartStaves(self) -> list[stream.PartStaff]: | ||
| ''' | ||
| Take a `Part` with multiple staves and make them a set of `PartStaff` objects. | ||
|
|
@@ -2247,7 +2238,7 @@ def adjustTimeAttributesFromMeasure(self, m: stream.Measure): | |
| else: | ||
| self.lastMeasureWasShort = False | ||
|
|
||
| self.lastMeasureOffset = opFrac(self.lastMeasureOffset + mOffsetShift) | ||
|
|
||
| def applyMultiMeasureRest(self, r: note.Rest): | ||
| ''' | ||
|
|
@@ -2567,19 +2558,8 @@ def parse(self): | |
| if methName is not None: | ||
| meth = getattr(self, methName) | ||
| meth(mxObj) | ||
| for v in self.stream[stream.Voice]: | ||
| v.coreElementsChanged() | ||
| self.stream.coreElementsChanged() | ||
|
|
||
| if (self.restAndNoteCount['rest'] == 1 | ||
|
|
@@ -2602,7 +2582,7 @@ def xmlBackup(self, mxObj: ET.Element): | |
| >>> mxBackup = EL('<backup><duration>100</duration></backup>') | ||
| >>> MP.xmlBackup(mxBackup) | ||
| >>> MP.offsetMeasureNote | ||
| Fraction(9979, 10000) | ||
|
|
||
| >>> MP.xmlBackup(mxBackup) | ||
| >>> MP.offsetMeasureNote | ||
|
|
@@ -2611,7 +2591,8 @@ def xmlBackup(self, mxObj: ET.Element): | |
| mxDuration = mxObj.find('duration') | ||
| if durationText := strippedText(mxDuration): | ||
| 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 | ||
|
|
@@ -2624,19 +2605,8 @@ def xmlForward(self, mxObj: ET.Element): | |
| mxDuration = mxObj.find('duration') | ||
| if durationText := strippedText(mxDuration): | ||
| change = opFrac(float(durationText) / self.divisions) | ||
| # Allow overfilled measures for now -- TODO(someday): warn? | ||
| self.offsetMeasureNote = opFrac(self.offsetMeasureNote + change) | ||
|
|
||
| def xmlPrint(self, mxPrint: ET.Element): | ||
| ''' | ||
|
|
@@ -2795,8 +2765,7 @@ def xmlToNote(self, mxNote: ET.Element) -> None: | |
| self.nLast = c # update | ||
|
|
||
| # only increment Chords after completion | ||
| self.offsetMeasureNote = opFrac(self.offsetMeasureNote + offsetIncrement) | ||
|
|
||
| def xmlToChord(self, mxNoteList: list[ET.Element]) -> chord.ChordBase: | ||
| # noinspection PyShadowingNames | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.