Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ugly MusicXML durations & offsets #1632

Merged
merged 5 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion music21/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
'''
from __future__ import annotations

__version__ = '9.2.0b1'
__version__ = '9.2.0b2'

def get_version_tuple(vv):
v = vv.split('.')
Expand Down
2 changes: 1 addition & 1 deletion music21/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<class 'music21.base.Music21Object'>
>>> music21.VERSION_STR
'9.2.0b1'
'9.2.0b2'
Alternatively, after doing a complete import, these classes are available
under the module "base":
Expand Down
43 changes: 36 additions & 7 deletions music21/musicxml/test_xmlToM21.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ def testExceptionMessage(self):
mxScorePart = self.EL('<score-part><part-name>Elec.</part-name></score-part>')
mxPart = self.EL('<part><measure><note><type>thirty-tooth</type></note></measure></part>')

PP = PartParser(mxPart=mxPart, mxScorePart=mxScorePart)
PP.partId = '1'
pp = PartParser(mxPart=mxPart, mxScorePart=mxScorePart)
pp.partId = '1'

msg = 'In part (Elec.), measure (0): found unknown MusicXML type: thirty-tooth'
with self.assertRaises(MusicXMLImportException) as error:
PP.parse()
pp.parse()
self.assertEqual(str(error.exception), msg)

def testBarRepeatConversion(self):
Expand Down Expand Up @@ -1478,21 +1478,21 @@ def testImportUnpitchedPercussion(self):
</score-part>
'''

PP = PartParser()
pp = PartParser()
mxScorePart = EL(scorePart)
tmb = PP.getDefaultInstrument(mxScorePart)
tmb = pp.getDefaultInstrument(mxScorePart)
self.assertIsInstance(tmb, instrument.Tambourine)
self.assertEqual(tmb.percMapPitch, 54) # 1-indexed

# An instrument music21 doesn't have yet (Cabasa):
scorePart = scorePart.replace('Tambourine', 'Cabasa')
scorePart = scorePart.replace('Tamb.', 'Cab.')
scorePart = scorePart.replace('55', '70') # 1-indexed
PP = PartParser()
pp = PartParser()
mxScorePart = EL(scorePart)
msg = '69 does not map to a valid instrument!'
with self.assertWarnsRegex(MusicXMLWarning, msg):
unp = PP.getDefaultInstrument(mxScorePart)
unp = pp.getDefaultInstrument(mxScorePart)
self.assertIsInstance(unp, instrument.UnpitchedPercussion)
self.assertEqual(unp.percMapPitch, 69)

Expand All @@ -1504,6 +1504,35 @@ def testImportImplicitMeasureNumber(self):
m = s[stream.Measure].first()
self.assertIs(m.showNumber, stream.enums.ShowNumber.NEVER)

def testAdjustTimeAttributesFromMeasure(self):
# Ignore import artifacts:
d = duration.Duration(3 + 3 / 480)
m = stream.Measure([meter.TimeSignature('6/8'), note.Note(duration=d)])
pp = PartParser()
pp.lastMeasureOffset = 21.0
pp.setLastMeasureInfo(m)
with self.assertWarns(MusicXMLWarning):
pp.adjustTimeAttributesFromMeasure(m)
self.assertEqual(pp.lastMeasureOffset, 24.0)

# Keep 'round' overful measures and extremely overful measures, as they were
# likely intentional.
d = duration.Duration(3.125)
m = stream.Measure([meter.TimeSignature('6/8'), note.Note(duration=d)])
pp = PartParser()
pp.lastMeasureOffset = 21.0
pp.setLastMeasureInfo(m)
pp.adjustTimeAttributesFromMeasure(m)
self.assertEqual(pp.lastMeasureOffset, 24.125)

d = duration.Duration(4.0)
m = stream.Measure([meter.TimeSignature('6/8'), note.Note(duration=d)])
pp = PartParser()
pp.lastMeasureOffset = 21.0
pp.setLastMeasureInfo(m)
pp.adjustTimeAttributesFromMeasure(m)
self.assertEqual(pp.lastMeasureOffset, 25.0)


if __name__ == '__main__':
import music21
Expand Down
121 changes: 66 additions & 55 deletions music21/musicxml/xmlToM21.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from music21 import duration
from music21 import dynamics
from music21.common.enums import OrnamentDelay
from music21.common.numberTools import opFrac, nearestMultiple
from music21 import editorial
from music21 import environment
from music21 import exceptions21
Expand Down Expand Up @@ -1624,10 +1625,10 @@ def getDefaultInstrument(self, mxScorePart: ET.Element | None = None) -> instrum
... + '</midi-instrument>'
... + '</score-part>')
>>> from xml.etree.ElementTree import fromstring as EL
>>> PP = musicxml.xmlToM21.PartParser()
>>> pp = musicxml.xmlToM21.PartParser()

>>> mxScorePart = EL(scorePart)
>>> i = PP.getDefaultInstrument(mxScorePart)
>>> i = pp.getDefaultInstrument(mxScorePart)
>>> i
<music21.instrument.Instrument ': Instrument 4'>
>>> i.instrumentName
Expand All @@ -1646,10 +1647,10 @@ def getDefaultInstrument(self, mxScorePart: ET.Element | None = None) -> instrum
... + '</midi-instrument>'
... + '</score-part>')
>>> from xml.etree.ElementTree import fromstring as EL
>>> PP = musicxml.xmlToM21.PartParser()
>>> pp = musicxml.xmlToM21.PartParser()

>>> mxScorePart = EL(scorePart)
>>> i = PP.getDefaultInstrument(mxScorePart)
>>> i = pp.getDefaultInstrument(mxScorePart)
>>> i
<music21.instrument.Trumpet ': C Trumpet'>
>>> i.instrumentName
Expand Down Expand Up @@ -2066,15 +2067,15 @@ def setLastMeasureInfo(self, m: stream.Measure):
Also sets self.lastTimeSignature from the timeSignature found in
the measure, if any.

>>> PP = musicxml.xmlToM21.PartParser()
>>> pp = musicxml.xmlToM21.PartParser()

Here are the defaults:

>>> PP.lastMeasureNumber
>>> pp.lastMeasureNumber
0
>>> PP.lastNumberSuffix is None
>>> pp.lastNumberSuffix is None
True
>>> PP.lastTimeSignature is None
>>> pp.lastTimeSignature is None
True

After setLastMeasureInfo:
Expand All @@ -2083,15 +2084,15 @@ def setLastMeasureInfo(self, m: stream.Measure):
>>> m.numberSuffix = 'b'
>>> ts38 = meter.TimeSignature('3/8')
>>> m.timeSignature = ts38
>>> PP.setLastMeasureInfo(m)
>>> pp.setLastMeasureInfo(m)

>>> PP.lastMeasureNumber
>>> pp.lastMeasureNumber
4
>>> PP.lastNumberSuffix
>>> pp.lastNumberSuffix
'b'
>>> PP.lastTimeSignature
>>> pp.lastTimeSignature
<music21.meter.TimeSignature 3/8>
>>> PP.lastTimeSignature is ts38
>>> pp.lastTimeSignature is ts38
True

Note that if there was no timeSignature defined in m,
Expand All @@ -2100,25 +2101,25 @@ def setLastMeasureInfo(self, m: stream.Measure):
after the first measure there's going to be routines
that need some sort of time signature:

>>> PP2 = musicxml.xmlToM21.PartParser()
>>> pp2 = musicxml.xmlToM21.PartParser()
>>> m2 = stream.Measure(number=2)
>>> PP2.setLastMeasureInfo(m2)
>>> PP2.lastTimeSignature
>>> pp2.setLastMeasureInfo(m2)
>>> pp2.lastTimeSignature
<music21.meter.TimeSignature 4/4>


For obscure reasons relating to how Finale gives suffixes
to unnumbered measures, if a measure has the same number
as the lastMeasureNumber, the lastNumberSuffix is not updated:

>>> PP3 = musicxml.xmlToM21.PartParser()
>>> PP3.lastMeasureNumber = 10
>>> PP3.lastNumberSuffix = 'X1'
>>> pp3 = musicxml.xmlToM21.PartParser()
>>> pp3.lastMeasureNumber = 10
>>> pp3.lastNumberSuffix = 'X1'

>>> m10 = stream.Measure(number=10)
>>> m10.numberSuffix = 'X2'
>>> PP3.setLastMeasureInfo(m10)
>>> PP3.lastNumberSuffix
>>> pp3.setLastMeasureInfo(m10)
>>> pp3.lastNumberSuffix
'X1'
'''
if m.number == self.lastMeasureNumber:
Expand Down Expand Up @@ -2156,22 +2157,22 @@ def adjustTimeAttributesFromMeasure(self, m: stream.Measure):
>>> m = stream.Measure([meter.TimeSignature('4/4'), harmony.ChordSymbol('C7')])
>>> m.highestTime
0.0
>>> PP = musicxml.xmlToM21.PartParser()
>>> PP.setLastMeasureInfo(m)
>>> PP.adjustTimeAttributesFromMeasure(m)
>>> pp = musicxml.xmlToM21.PartParser()
>>> pp.setLastMeasureInfo(m)
>>> pp.adjustTimeAttributesFromMeasure(m)
>>> m.highestTime
4.0
>>> PP.lastMeasureWasShort
>>> pp.lastMeasureWasShort
False

Incomplete final measure:

>>> m = stream.Measure([meter.TimeSignature('6/8'), note.Note(), note.Note()])
>>> m.offset = 24.0
>>> PP = musicxml.xmlToM21.PartParser()
>>> PP.lastMeasureOffset = 21.0
>>> PP.setLastMeasureInfo(m)
>>> PP.adjustTimeAttributesFromMeasure(m)
>>> pp = musicxml.xmlToM21.PartParser()
>>> pp.lastMeasureOffset = 21.0
>>> pp.setLastMeasureInfo(m)
>>> pp.adjustTimeAttributesFromMeasure(m)
>>> m.paddingRight
1.0
'''
Expand All @@ -2190,9 +2191,25 @@ def adjustTimeAttributesFromMeasure(self, m: stream.Measure):
else:
lastTimeSignatureQuarterLength = 4.0 # sensible default.

if mHighestTime >= lastTimeSignatureQuarterLength:
if mHighestTime == lastTimeSignatureQuarterLength:
mOffsetShift = mHighestTime

elif mHighestTime > lastTimeSignatureQuarterLength:
diff = mHighestTime - lastTimeSignatureQuarterLength
tol = 1e-6
# If the measure is overfull by a "round" amount, assume that it was intended
# otherwise it was likely the result of malformed MusicXML.
if (diff > 0.5
or nearestMultiple(diff, 0.0625)[1] < tol
or nearestMultiple(diff, 1 / 12)[1] < tol):
mOffsetShift = mHighestTime
else:
mOffsetShift = lastTimeSignatureQuarterLength
warnings.warn(
f'Warning: measure {m.number} in part {self.stream.partName}'
f'is overfull: {mHighestTime} > {lastTimeSignatureQuarterLength},'
f'assuming {mOffsetShift} is correct.',
MusicXMLWarning
)
elif (mHighestTime == 0.0
and not m.recurse().notesAndRests.getElementsNotOfClass('Harmony')
):
Expand All @@ -2204,7 +2221,6 @@ def adjustTimeAttributesFromMeasure(self, m: stream.Measure):
m.insert(0.0, r)
mOffsetShift = lastTimeSignatureQuarterLength
self.lastMeasureWasShort = False

else: # use time signature
# for the first measure, this may be a pickup
# must detect this when writing, as next measures offsets will be
Expand All @@ -2216,7 +2232,6 @@ def adjustTimeAttributesFromMeasure(self, m: stream.Measure):
# environLocal.printDebug(['incompletely filled Measure found on musicxml
# import; interpreting as an anacrusis:', 'paddingLeft:', m.paddingLeft])
mOffsetShift = mHighestTime

else:
mOffsetShift = mHighestTime # lastTimeSignatureQuarterLength
if self.lastMeasureWasShort is True:
Expand All @@ -2239,39 +2254,39 @@ def applyMultiMeasureRest(self, r: note.Rest):
'''
If there is an active MultiMeasureRestSpanner, add the Rest, r, to it:

>>> PP = musicxml.xmlToM21.PartParser()
>>> pp = musicxml.xmlToM21.PartParser()
>>> mmrSpanner = spanner.MultiMeasureRest()
>>> mmrSpanner
<music21.spanner.MultiMeasureRest 0 measures>

>>> PP.activeMultiMeasureRestSpanner = mmrSpanner
>>> PP.multiMeasureRestsToCapture = 2
>>> pp.activeMultiMeasureRestSpanner = mmrSpanner
>>> pp.multiMeasureRestsToCapture = 2
>>> r1 = note.Rest(type='whole', id='r1')
>>> PP.applyMultiMeasureRest(r1)
>>> PP.multiMeasureRestsToCapture
>>> pp.applyMultiMeasureRest(r1)
>>> pp.multiMeasureRestsToCapture
1
>>> PP.activeMultiMeasureRestSpanner
>>> pp.activeMultiMeasureRestSpanner
<music21.spanner.MultiMeasureRest 1 measure>

>>> PP.activeMultiMeasureRestSpanner is mmrSpanner
>>> pp.activeMultiMeasureRestSpanner is mmrSpanner
True
>>> PP.stream.show('text') # Nothing...
>>> pp.stream.show('text') # Nothing...

>>> r2 = note.Rest(type='whole', id='r2')
>>> PP.applyMultiMeasureRest(r2)
>>> PP.multiMeasureRestsToCapture
>>> pp.applyMultiMeasureRest(r2)
>>> pp.multiMeasureRestsToCapture
0
>>> PP.activeMultiMeasureRestSpanner is None
>>> pp.activeMultiMeasureRestSpanner is None
True

# spanner added to stream

>>> PP.stream.show('text')
>>> pp.stream.show('text')
{0.0} <music21.spanner.MultiMeasureRest 2 measures>

>>> r3 = note.Rest(type='whole', id='r3')
>>> PP.applyMultiMeasureRest(r3)
>>> PP.stream.show('text')
>>> pp.applyMultiMeasureRest(r3)
>>> pp.stream.show('text')
{0.0} <music21.spanner.MultiMeasureRest 2 measures>

'''
Expand Down Expand Up @@ -2596,9 +2611,7 @@ def xmlBackup(self, mxObj: ET.Element):
'''
mxDuration = mxObj.find('duration')
if durationText := strippedText(mxDuration):
change = common.numberTools.opFrac(
float(durationText) / self.divisions
)
change = opFrac(float(durationText) / self.divisions)
self.offsetMeasureNote -= change
# check for negative offsets produced by
# musicxml durations with float rounding issues
Expand All @@ -2611,9 +2624,7 @@ def xmlForward(self, mxObj: ET.Element):
'''
mxDuration = mxObj.find('duration')
if durationText := strippedText(mxDuration):
change = common.numberTools.opFrac(
float(durationText) / self.divisions
)
change = opFrac(float(durationText) / self.divisions)

# Create hidden rest (in other words, a spacer)
# old Finale documents close incomplete final measures with <forward>
Expand Down Expand Up @@ -3578,7 +3589,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

Expand Down Expand Up @@ -5510,7 +5521,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
Expand Down
Loading