Repository: cuthbertLab/music21
Pull request #1142 (2021-09-28 08:37:13)

Hammer-on and Pull-off as Spanners


Patch:
diff --git a/music21/articulations.py b/music21/articulations.py
index 65ce60801..fdb00874d 100644
--- a/music21/articulations.py
+++ b/music21/articulations.py
@@ -84,6 +84,7 @@ from music21 import common
 from music21.common.classTools import tempAttribute
 from music21 import environment
 from music21 import style
+from music21 import spanner
 
 if t.TYPE_CHECKING:
     from music21 import interval
@@ -568,10 +569,16 @@ class FrettedPluck(FretIndication, Fingering):
     '''
     pass
 
-class HammerOn(FretIndication):
+class HammerOn(spanner.Spanner, TechnicalIndication):
+    '''
+    A hammer-on represented as a spanner between two Notes.
+    '''
     pass
 
-class PullOff(FretIndication):
+class PullOff(spanner.Spanner, TechnicalIndication):
+    '''
+    A pull-off represented as a spanner between two Notes.
+    '''
     pass
 
 class FretBend(FretIndication):
@@ -681,4 +688,3 @@ _DOC_ORDER = [Articulation]
 if __name__ == '__main__':
     import music21
     music21.mainTest(Test)
-
diff --git a/music21/musicxml/m21ToXml.py b/music21/musicxml/m21ToXml.py
index e7025bb35..2076b7fb8 100644
--- a/music21/musicxml/m21ToXml.py
+++ b/music21/musicxml/m21ToXml.py
@@ -3141,7 +3141,6 @@ class MeasureExporter(XMLExporterBase):
             ('RehearsalMark', 'rehearsalMarkToXml'),
         ]
     )
-
     # these need to be wrapped in an attributes tag if not at the beginning of the measure.
     wrapAttributeMethodClasses = OrderedDict(
         [('Clef', 'clefToXml'),
@@ -3693,6 +3692,80 @@ class MeasureExporter(XMLExporterBase):
 
         return notations
 
+    def objectAttachedSpannersToTechnicals(self, obj, objectSpannerBundle=None):
+        '''
+        return a list of <technical> from spanners related to the object that should appear
+        in the technical tag of the notations tag (hammer-on, pull-off etc.)
+
+        >>> n0 = note.Note('C')
+        >>> n1 = note.Note('D')
+        >>> n2 = note.Note('F')
+        >>> n3 = note.Note('E')
+        >>> hammerOn01 = articulations.HammerOn([n0, n1])
+        >>> pullOff23 = articulations.PullOff([n2, n3])
+        >>> m = stream.Measure()
+        >>> m.insert(0, hammerOn01)
+        >>> m.insert(0, pullOff23)
+        >>> m.append(n0)
+        >>> m.append(n1)
+        >>> m.append(n2)
+        >>> m.append(n3)
+        >>> mex = musicxml.m21ToXml.MeasureExporter(m)
+        >>> out = mex.objectAttachedSpannersToTechnicals(n0, m.spannerBundle)
+        >>> out
+        [<Element 'hammer-on' at 0x102857f40>]
+        >>> mex.dump(out[0])
+        <hammer-on number="1" type="start">H</hammer-on>
+        >>> out = mex.objectAttachedSpannersToTechnicals(n3, m.spannerBundle)
+        >>> mex.dump(out[0])
+        <pull-off number="1" type="stop" />
+
+        OMIT_FROM_DOCS
+
+        The other permutations of class and first/last:
+
+        >>> out = mex.objectAttachedSpannersToTechnicals(n1, m.spannerBundle)
+        >>> mex.dump(out[0])
+        <hammer-on number="1" type="stop" />
+        >>> out = mex.objectAttachedSpannersToTechnicals(n2, m.spannerBundle)
+        >>> mex.dump(out[0])
+        <pull-off number="1" type="start">P</pull-off>
+        '''
+        technicals = []
+        if objectSpannerBundle is not None:
+            sb = objectSpannerBundle
+        else:
+            sb = self.objectSpannerBundle
+
+        if not sb:
+            return technicals
+
+        for su in sb.getByClass(articulations.HammerOn):
+            mxHammerOn = Element('hammer-on')
+            if su.isFirst(obj):
+                mxHammerOn.set('type', 'start')
+                mxHammerOn.text = 'H'
+            elif su.isLast(obj):
+                mxHammerOn.set('type', 'stop')
+            else:
+                continue
+            mxHammerOn.set('number', '1')
+            technicals.append(mxHammerOn)
+
+        for su in sb.getByClass(articulations.PullOff):
+            mxPullOff = Element('pull-off')
+            if su.isFirst(obj):
+                mxPullOff.set('type', 'start')
+                mxPullOff.text = 'P'
+            elif su.isLast(obj):
+                mxPullOff.set('type', 'stop')
+            else:
+                continue
+            mxPullOff.set('number', '1')
+            technicals.append(mxPullOff)
+
+        return technicals
+
     def appendArpeggioMarkSpannersToNotations(
         self,
         obj: base.Music21Object,
@@ -4695,7 +4768,8 @@ class MeasureExporter(XMLExporterBase):
                 else:
                     notations.append(mxExpression)
 
-        # apply all articulations apart from fingerings only to first note of chord
+        # apply all articulations apart from fingerings and hammer-on/pull-off
+        # only to first note of chord
         applicableArticulations = []
         fingeringNumber = 0
         for a in chordOrNote.articulations:
@@ -4704,7 +4778,14 @@ class MeasureExporter(XMLExporterBase):
                     applicableArticulations.append(a)
                 fingeringNumber += 1
             elif isSingleNoteOrFirstInChord:
-                applicableArticulations.append(a)
+                # Ignore hammer-on/pull-off:
+                # are written from their Spanner representation instead.
+                # It's an anti-pattern to find spanners in the articulations
+                # array, and the musicxml importer doesn't put them here,
+                # but it's a potential point of user confusion, so we guard
+                # against it here to avoid writing out superfluous <other-technical>
+                if not isinstance(a, (articulations.HammerOn, articulations.PullOff)):
+                    applicableArticulations.append(a)
 
         for artObj in applicableArticulations:
             if isinstance(artObj, articulations.Pizzicato):
@@ -4714,6 +4795,7 @@ class MeasureExporter(XMLExporterBase):
             if isinstance(artObj, articulations.TechnicalIndication):
                 if mxTechnicalMark is None:
                     mxTechnicalMark = Element('technical')
+
                 mxTechnicalMark.append(self.articulationToXmlTechnical(artObj))
             else:
                 if mxArticulations is None:
@@ -4739,6 +4821,17 @@ class MeasureExporter(XMLExporterBase):
         # TODO: glissando
         # TODO: slide
 
+        if isSingleNoteOrFirstInChord and chordParent is not None:
+            spannerTechnicals = self.objectAttachedSpannersToTechnicals(chordParent)
+        elif chordParent is not None:
+            spannerTechnicals = None
+        else:
+            spannerTechnicals = self.objectAttachedSpannersToTechnicals(n)
+        if spannerTechnicals:
+            if mxTechnicalMark is None:
+                mxTechnicalMark = Element('technical')
+            mxTechnicalMark.extend(spannerTechnicals)
+
         for x in (mxArticulations,
                   mxTechnicalMark,
                   mxOrnaments):
@@ -5185,14 +5278,6 @@ class MeasureExporter(XMLExporterBase):
         >>> mxOther = MEX.articulationToXmlTechnical(g)
         >>> MEX.dump(mxOther)
         <other-technical>unda maris</other-technical>
-
-        Same with technical marks not yet supported.
-        TODO: support HammerOn, PullOff, Bend, Hole, Arrow.
-
-        >>> h = articulations.HammerOn()
-        >>> mxOther = MEX.articulationToXmlTechnical(h)
-        >>> MEX.dump(mxOther)
-        <other-technical />
         '''
         # these technical have extra information
         # TODO: hammer-on
@@ -5209,7 +5294,7 @@ class MeasureExporter(XMLExporterBase):
             musicXMLTechnicalName = 'other-technical'
 
         # TODO: support additional technical marks listed above
-        if musicXMLTechnicalName in ('hammer-on', 'pull-off', 'bend', 'hole', 'arrow'):
+        if musicXMLTechnicalName in ('bend', 'hole', 'arrow'):
             musicXMLTechnicalName = 'other-technical'
 
         mxTechnicalMark = Element(musicXMLTechnicalName)
diff --git a/music21/musicxml/xmlObjects.py b/music21/musicxml/xmlObjects.py
index 5813209f5..ed983720f 100644
--- a/music21/musicxml/xmlObjects.py
+++ b/music21/musicxml/xmlObjects.py
@@ -67,8 +67,10 @@ TECHNICAL_MARKS = OrderedDict([('up-bow', articulations.UpBow),
                                ('stopped', articulations.Stopped),
                                ('snap-pizzicato', articulations.SnapPizzicato),
                                ('string', articulations.StringIndication),
-                               ('hammer-on', articulations.HammerOn),
-                               ('pull-off', articulations.PullOff),
+                               # hammer-on and pull-off not implemented because handled
+                               # in method objectAttachedSpannersToTechnicals of m21ToXml.py
+                               # ('hammer-on', articulations.HammerOn),
+                               # ('pull-off', articulations.PullOff),
                                # bend not implemented because it needs many subcomponents
                                # ('bend', articulations.FretBend),
                                ('tap', articulations.FretTap),
diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py
index f5648f495..1b789fdb8 100644
--- a/music21/musicxml/xmlToM21.py
+++ b/music21/musicxml/xmlToM21.py
@@ -4159,6 +4159,12 @@ class MeasureParser(XMLParserBase):
                                     )
             self.setColor(mxObj, slur)
 
+        for mxObj in mxNotations.findall('technical/hammer-on'):
+            self.xmlOneSpanner(mxObj, n, articulations.HammerOn)
+
+        for mxObj in mxNotations.findall('technical/pull-off'):
+            self.xmlOneSpanner(mxObj, n, articulations.PullOff)
+
         for tagSearch in ('glissando', 'slide'):
             for mxObj in mxNotations.findall(tagSearch):
                 gliss = self.xmlOneSpanner(mxObj, n, spanner.Glissando)
diff --git a/music21/spanner.py b/music21/spanner.py
index c2b3c79f1..1cb8d5e55 100644
--- a/music21/spanner.py
+++ b/music21/spanner.py
@@ -2744,6 +2744,29 @@ class Test(unittest.TestCase):
         slList = sl.getSpannedElementIds()
         self.assertEqual(idList, slList)
 
+    def testHammerOnPullOff(self):
+        from music21 import converter
+        from music21.musicxml import testPrimitive
+
+        s = converter.parse(testPrimitive.notations32a)
+
+        num_hammer_on = len(s.flatten().getElementsByClass('HammerOn'))
+        num_pull_off = len(s.flatten().getElementsByClass('PullOff'))
+        self.assertEqual(num_hammer_on, 1)
+        self.assertEqual(num_pull_off, 1)
+
+        hammer_on = s.flatten().getElementsByClass('HammerOn')[0]
+        hammer_n0 = hammer_on.getSpannedElements()[0]
+        hammer_n1 = hammer_on.getSpannedElements()[1]
+        self.assertEqual(hammer_n0.getSpannerSites()[0], hammer_on)
+        self.assertEqual(hammer_n1.getSpannerSites()[0], hammer_on)
+
+        pull_off = s.flatten().getElementsByClass('PullOff')[0]
+        pull_n0 = pull_off.getSpannedElements()[0]
+        pull_n1 = pull_off.getSpannedElements()[1]
+        self.assertEqual(pull_n0.getSpannerSites()[0], pull_off)
+        self.assertEqual(pull_n1.getSpannerSites()[0], pull_off)
+
 
 # ------------------------------------------------------------------------------
 # define presented order in documentation


Test patch:
diff --git a/music21/musicxml/test_m21ToXml.py b/music21/musicxml/test_m21ToXml.py
index ad7f0d770..3caca37ad 100644
--- a/music21/musicxml/test_m21ToXml.py
+++ b/music21/musicxml/test_m21ToXml.py
@@ -519,6 +519,13 @@ class Test(unittest.TestCase):
         n = note.Note()
         a = articulations.StringIndication()
         n.articulations.append(a)
+        h = articulations.HammerOn()
+        # Appending a spanner such as HammerOn to the articulations
+        # array is contrary to the documentation and contrary to the
+        # behavior of the musicxml importer, but we're testing it here
+        # anyway to just ensure that *IF* a user decides to do this themselves,
+        # we don't create a superfluous <other-technical> tag on export.
+        n.articulations.append(h)
 
         # Legal values for StringIndication begin at 1
         self.assertEqual(a.number, 0)
@@ -526,6 +533,7 @@ class Test(unittest.TestCase):
         gex = GeneralObjectExporter(n)
         tree = et_fromstring(gex.parse().decode('utf-8'))
         self.assertIsNone(tree.find('.//string'))
+        self.assertIsNone(tree.find('.//other-technical'))
 
     def testMeasurePadding(self):
         s = stream.Score([converter.parse('tinyNotation: 4/4 c4')])