From 37acfe7f524e9552f9c3d54948aaaf26dfc6b94c Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 9 Oct 2026 03:33:04 -1000 Subject: [PATCH 1/2] Cache parsed Pitch names The .name setter remembers each string it parses as (step, accidental string, octave) in a module-level dict, cleared at 1000 entries. A remembered name sets the attributes directly, makes a new Accidental (they are mutable, so never shared), and informs the Note once. Names that fail to parse are not remembered and raise as before. Best of 7, master -> branch: Pitch('D5') 543 -> 230 ns Pitch('F#4') 691 -> 319 ns p.name = 'F#4' 556 -> 172 ns Note('F#4') 1812 -> 1382 ns AI-assisted (Claude) --- music21/pitch.py | 41 +++++++++++++++++++++++++++++++++++++- music21/test/test_pitch.py | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/music21/pitch.py b/music21/pitch.py index 41bf57f0f..8d427281c 100644 --- a/music21/pitch.py +++ b/music21/pitch.py @@ -1653,6 +1653,12 @@ def fullName(self) -> str: # ------------------------------------------------------------------------------ +# strings that Pitch.name parsed: (step, accidental string or None, octave or None). +# Holds no Accidental objects, since those are mutable. Cleared when full. +_pitchNameCache: dict[str, tuple[StepName, str|None, int|None]] = {} +_pitchNameCacheSize = 1000 + + # tried as SlottedObjectMixin -- made creation time slower! Not worth the restrictions class Pitch(prebase.ProtoM21Object): ''' @@ -2846,6 +2852,32 @@ def name(self, usrStr: str) -> None: Set name, which may be provided with or without octave values. C4 or D-3 are both accepted. ''' + try: + step, accidentalStr, octave = _pitchNameCache[usrStr] + except (KeyError, TypeError): # TypeError: not hashable, so not a str + self._parseName(usrStr) + return + self._step = step + self.spellingIsInferred = False + self._accidental = (None if accidentalStr is None + else Accidental(accidentalStr)) + if octave is not None: + self._octave = octave + self.informClient() + + def _parseName(self, usrStr: str) -> None: + ''' + The `.name` setter for a string not yet seen: parses it, sets step, + accidental, and octave, and remembers the result for next time. + + >>> p = pitch.Pitch() + >>> p._parseName(' f#5') + >>> p + + >>> pitch._pitchNameCache[' f#5'] + ('F', '#', 5) + ''' + cacheKey = usrStr try: usrStr = usrStr.strip() except AttributeError: @@ -2866,6 +2898,7 @@ def name(self, usrStr: str) -> None: octNot.append(char) usrStr = ''.join(octNot) octFoundStr = ''.join(octFound) + accidentalStr: str|None = None # we have nothing but pitch specification if len(usrStr) == 1: self.step = usrStr # type: ignore @@ -2873,14 +2906,20 @@ def name(self, usrStr: str) -> None: # assume everything following pitch is accidental specification elif len(usrStr) > 1: self.step = usrStr[0] # type: ignore - self.accidental = Accidental(usrStr[1:]) + accidentalStr = usrStr[1:] + self.accidental = Accidental(accidentalStr) else: raise PitchException(f'Cannot make a name out of {usrStr!r}') + octave: int|None = None if octFoundStr: # bool('0') == True, so okay octave = int(octFoundStr) self.octave = octave + if len(_pitchNameCache) >= _pitchNameCacheSize: + _pitchNameCache.clear() + _pitchNameCache[cacheKey] = (self._step, accidentalStr, octave) + @property def unicodeName(self) -> str: ''' diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index 4037e92f5..d4aa977de 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -142,6 +142,43 @@ def testNameSetting(self): ): p.name = 32 + def testNameCache(self): + ''' + A name is parsed once and remembered; names that cannot be parsed are not. + ''' + with mock.patch.dict(pitch._pitchNameCache, clear=True): + first = Pitch('E-6') + self.assertEqual(pitch._pitchNameCache, {'E-6': ('E', '-', 6)}) + second = Pitch('E-6') + self.assertEqual(second.nameWithOctave, 'E-6') + # Accidentals can be changed, so each Pitch gets its own + self.assertIsNot(second.accidental, first.accidental) + + with self.assertRaises(AccidentalException): + Pitch('C$') + with self.assertRaisesRegex(ValueError, 'must be a string'): + second.name = ['C4'] # not hashable + self.assertEqual(list(pitch._pitchNameCache), ['E-6']) + + # cleared when full + with mock.patch.object(pitch, '_pitchNameCacheSize', 1): + Pitch('F4') + self.assertEqual(list(pitch._pitchNameCache), ['F4']) + + def testRememberedNameInformsNoteOnce(self): + ''' + Setting a remembered name without an octave keeps the octave and the + microtone, makes the spelling explicit, and tells the Note once. + ''' + Pitch('B-') + n = note.Note(73) # C#5, spelling inferred + n.pitch.microtone = 20 + with mock.patch.object(n, 'pitchChanged') as pitchChanged: + n.pitch.name = 'B-' + pitchChanged.assert_called_once() + self.assertEqual(str(n.pitch), 'B-5(+20c)') + self.assertFalse(n.pitch.spellingIsInferred) + def testInitShortcutsMatchParsing(self): # 'C', 'C4', and step= skip the name and step setters self.assertEqual(Pitch('C'), Pitch('c')) From ebc27ea393eca89418933eeb8746bd51f209fd01 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Fri, 9 Oct 2026 17:56:08 -1000 Subject: [PATCH 2/2] Name cache: parse into the cache, then set once; early return for style _parseName is now _cacheParsedName: it only parses and remembers (step, accidental string, octave). The .name setter reads the cache after a miss too, so it sets the attributes and informs the Note once even the first time, and a name that fails to parse leaves the pitch unchanged (before, D$ changed the step to D and then raised). Names over 100 characters raise a ValueError, so the cache cannot hold huge strings. setStyleAttributes returns early for a tag with no attributes (most , , tags): natural 1378 -> 491 ns; about 1-3% on whole MusicXML files. AI-assisted (Claude) --- music21/musicxml/xmlToM21.py | 3 +++ music21/pitch.py | 41 ++++++++++++++++++------------------ music21/test/test_pitch.py | 17 +++++++++------ 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/music21/musicxml/xmlToM21.py b/music21/musicxml/xmlToM21.py index de35ee2e5..07d8c39eb 100644 --- a/music21/musicxml/xmlToM21.py +++ b/music21/musicxml/xmlToM21.py @@ -301,6 +301,9 @@ def setStyleAttributes(self, mxObject, m21Object, musicXMLNames, m21Names=None): >>> m21Obj.style.hideObjectOnPrint True ''' + if not mxObject.attrib: + return + if isinstance(m21Object, style.Style): stObj = m21Object else: diff --git a/music21/pitch.py b/music21/pitch.py index 67ab52e24..145c3d81d 100644 --- a/music21/pitch.py +++ b/music21/pitch.py @@ -2817,6 +2817,8 @@ def name(self) -> str: >>> a = pitch.Pitch('B---') >>> a.name 'B---' + + * Changed in v11: a name longer than 100 characters raises a ValueError. ''' if self.accidental is not None: return self.step + self.accidental.modifier @@ -2832,8 +2834,8 @@ def name(self, usrStr: str) -> None: try: step, accidentalStr, octave = _pitchNameCache[usrStr] except (KeyError, TypeError): # TypeError: not hashable, so not a str - self._parseName(usrStr) - return + self._cacheParsedName(usrStr) + step, accidentalStr, octave = _pitchNameCache[usrStr] self._step = step self.spellingIsInferred = False self._accidental = (None if accidentalStr is None @@ -2842,15 +2844,13 @@ def name(self, usrStr: str) -> None: self._octave = octave self.informClient() - def _parseName(self, usrStr: str) -> None: + def _cacheParsedName(self, usrStr: str) -> None: ''' - The `.name` setter for a string not yet seen: parses it, sets step, - accidental, and octave, and remembers the result for next time. + Parse a name not yet seen and remember its step, accidental, and + octave for the `.name` setter. Raises if it is not a name. >>> p = pitch.Pitch() - >>> p._parseName(' f#5') - >>> p - + >>> p._cacheParsedName(' f#5') >>> pitch._pitchNameCache[' f#5'] ('F', '#', 5) ''' @@ -2859,6 +2859,10 @@ def _parseName(self, usrStr: str) -> None: usrStr = usrStr.strip() except AttributeError: raise ValueError(f'Argument to name, {usrStr!r}, must be a string, not {type(usrStr)}.') + # remembered names are kept, so do not keep huge ones + if len(cacheKey) > 100: + raise ValueError( + f'Argument to name must be at most 100 characters, not {len(cacheKey)}.') # extract any numbers that may be octave designations octFound: list[str] = [] @@ -2875,27 +2879,24 @@ def _parseName(self, usrStr: str) -> None: octNot.append(char) usrStr = ''.join(octNot) octFoundStr = ''.join(octFound) - accidentalStr: str|None = None - # we have nothing but pitch specification - if len(usrStr) == 1: - self.step = usrStr # type: ignore - self.accidental = None + if not usrStr: + raise PitchException(f'Cannot make a name out of {usrStr!r}') + step = t.cast(StepName, usrStr[0].upper()) + if step not in STEPNAMES: + raise PitchException(f'Cannot make a step out of {step!r}') # assume everything following pitch is accidental specification - elif len(usrStr) > 1: - self.step = usrStr[0] # type: ignore + accidentalStr: str|None = None + if len(usrStr) > 1: accidentalStr = usrStr[1:] - self.accidental = Accidental(accidentalStr) - else: - raise PitchException(f'Cannot make a name out of {usrStr!r}') + Accidental(accidentalStr) # raises if it is not an accidental octave: int|None = None if octFoundStr: # bool('0') == True, so okay octave = int(octFoundStr) - self.octave = octave if len(_pitchNameCache) >= _pitchNameCacheSize: _pitchNameCache.clear() - _pitchNameCache[cacheKey] = (self._step, accidentalStr, octave) + _pitchNameCache[cacheKey] = (step, accidentalStr, octave) @property def unicodeName(self) -> str: diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index 165238677..6c57994ab 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -134,26 +134,31 @@ def testNameCache(self): # Accidentals can be changed, so each Pitch gets its own self.assertIsNot(second.accidental, first.accidental) + # a name that cannot be parsed changes nothing with self.assertRaises(AccidentalException): - Pitch('C$') + second.name = 'D$' + self.assertEqual(second.nameWithOctave, 'E-6') with self.assertRaisesRegex(ValueError, 'must be a string'): second.name = ['C4'] # not hashable + with self.assertRaisesRegex(ValueError, 'at most 100 characters'): + second.name = 'C4'.center(101) # padding counts self.assertEqual(list(pitch._pitchNameCache), ['E-6']) + self.assertEqual(Pitch('C4'.center(100)).nameWithOctave, 'C4') # cleared when full with mock.patch.object(pitch, '_pitchNameCacheSize', 1): Pitch('F4') self.assertEqual(list(pitch._pitchNameCache), ['F4']) - def testRememberedNameInformsNoteOnce(self): + def testNameSetterInformsNoteOnce(self): ''' - Setting a remembered name without an octave keeps the octave and the - microtone, makes the spelling explicit, and tells the Note once. + Setting a name without an octave, even one not seen before, keeps the + octave and the microtone, makes the spelling explicit, and tells the Note once. ''' - Pitch('B-') n = note.Note(73) # C#5, spelling inferred n.pitch.microtone = 20 - with mock.patch.object(n, 'pitchChanged') as pitchChanged: + with (mock.patch.dict(pitch._pitchNameCache, clear=True), + mock.patch.object(n, 'pitchChanged') as pitchChanged): n.pitch.name = 'B-' pitchChanged.assert_called_once() self.assertEqual(str(n.pitch), 'B-5(+20c)')