From 0c336eefc22c7ace10a8d83bece4686983506b0d Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Wed, 7 Oct 2026 19:11:43 -1000 Subject: [PATCH 1/4] Faster whole-number ps and MIDI pitches pitchClassToDefaultStepAlter gives the step and alter for each pitch class. _convertPsToStep, the ps setter, and Pitch(number) use it for whole numbers. The ps setter no longer builds a natural Accidental only to discard it, or a Microtone(0) (_microtone stays None, which every reader treats the same), and it assigns step, accidental, and octave directly, informing the client once instead of three times. Results are unchanged: p.midi = 60 still gives no accidental, and Pitch(60) still gives a natural. p.midi = 60 745 ns -> 165 ns p.midi = 61 836 ns -> 366 ns p.ps = 61.0 952 ns -> 345 ns Pitch(60) 658 ns -> 406 ns Pitch(midi=60) 906 ns -> 324 ns Note(60) 1779 ns -> 1482 ns Whole-file MIDI imports do not measurably change: pitches are a tiny part. AI-assisted (Claude) --- music21/pitch.py | 56 ++++++++++++++++++++++++++------------ music21/test/test_pitch.py | 31 +++++++++++++++++++++ 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/music21/pitch.py b/music21/pitch.py index a644e758c..f88abf957 100644 --- a/music21/pitch.py +++ b/music21/pitch.py @@ -63,6 +63,20 @@ 9: 'A', 11: 'B', } +pitchClassToDefaultStepAlter: tuple[tuple[StepName, int], ...] = ( + ('C', 0), # 0 + ('C', 1), # 1 + ('D', 0), # 2 + ('E', -1), # 3 + ('E', 0), # 4 + ('F', 0), # 5 + ('F', 1), # 6 + ('G', 0), # 7 + ('G', 1), # 8 + ('A', 0), # 9 + ('B', -1), # 10 + ('B', 0), # 11 +) STEPNAMES: set[StepName] = {'C', 'D', 'E', 'F', 'G', 'A', 'B'} # set STEP_TO_DNN_OFFSET: dict[StepName, int] = { 'C': 0, @@ -369,14 +383,9 @@ def _convertPsToStep( >>> pitch._convertPsToStep(59.9999999) ('C', , , 0) ''' - if isinstance(ps, int): - pc = ps % 12 - alter = 0.0 - micro = 0.0 - elif ps == int(ps): - pc = int(ps) % 12 - alter = 0.0 - micro = 0.0 + if isinstance(ps, int) or ps == int(ps): + step, wholeAlter = pitchClassToDefaultStepAlter[int(ps) % 12] + return step, Accidental(wholeAlter), Microtone(0), 0 else: # rounding here is essential ps = round(ps, PITCH_SPACE_SIG_DIGITS) @@ -1959,7 +1968,11 @@ def __init__(self, else: # is a number # is a midiNumber or a ps -- a float midiNumber # get step and accidental w/o octave - self.step, self._accidental = _convertPsToStep(name)[0:2] + if isinstance(name, int) or name == int(name): + self._step, alter = pitchClassToDefaultStepAlter[int(name) % 12] + self._accidental = Accidental(alter) + else: + self.step, self._accidental = _convertPsToStep(name)[0:2] self.spellingIsInferred = True if name >= 12: # is not a pitchClass self._octave = int(name / 12) - 1 @@ -2641,19 +2654,28 @@ def ps(self) -> float: @ps.setter def ps(self, value: int|float) -> None: - # can assign microtone here; will be either None or a Microtone object - self.step, acc, self._microtone, octShift = _convertPsToStep(value) - - # replace a natural with a None - if acc.name == 'natural': - self.accidental = None + if isinstance(value, int) or value == int(value): + wholePs = int(value) + self._step, alter = pitchClassToDefaultStepAlter[wholePs % 12] + # a natural is None + self._accidental = Accidental(alter) if alter else None + self._microtone = None + self._octave = wholePs // 12 - 1 else: - self.accidental = acc - self.octave = _convertPsToOct(value) + octShift + # can assign microtone here; will be either None or a Microtone object + self._step, acc, self._microtone, octShift = _convertPsToStep(value) + + # replace a natural with a None + if acc.name == 'natural': + self._accidental = None + else: + self._accidental = acc + self._octave = _convertPsToOct(value) + octShift # all ps settings must set implicit to True, as we do not know # what accidental this is self.spellingIsInferred = True + self.informClient() @property def midi(self) -> int: diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index 97607e8ca..92e9d62bd 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -131,6 +131,37 @@ def testInitShortcutsMatchParsing(self): with self.assertRaises(PitchException): Pitch(step='CD') + def testWholeNumberPsAndMidi(self): + ''' + Setting a whole-number .ps or .midi spells the pitch, resets the microtone, + marks the spelling inferred, and tells the Note. A natural is None. + ''' + names = ['C', 'C#', 'D', 'E-', 'E', 'F', 'F#', 'G', 'G#', 'A', 'B-', 'B'] + for ps in range(-13, 140): + for attribute, value in (('ps', ps), ('ps', float(ps)), ('midi', ps)): + if attribute == 'midi' and not 0 <= ps <= 127: + continue + with self.subTest(attribute=attribute, value=value): + n = note.Note('D~5') + n.pitch.microtone = 20 + n._cache['junk'] = 1 + setattr(n.pitch, attribute, value) + self.assertEqual(n.pitch.name, names[ps % 12]) + self.assertEqual(n.pitch.octave, ps // 12 - 1) + self.assertEqual(n.pitch.ps, ps) + if len(names[ps % 12]) == 1: + self.assertIsNone(n.pitch.accidental) + self.assertEqual(n.pitch.microtone.cents, 0) + self.assertTrue(n.pitch.spellingIsInferred) + self.assertEqual(n._cache, {}) + + # Pitch(number) keeps the natural + for midiNumber in range(12, 128): + p = Pitch(midiNumber) + self.assertEqual(p.nameWithOctave, f'{names[midiNumber % 12]}{midiNumber // 12 - 1}') + self.assertIsNotNone(p.accidental) + self.assertTrue(p.spellingIsInferred) + def testAccidentalImport(self): From d912098ba390de8273cbc7958c865020e386d0f2 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Wed, 7 Oct 2026 19:48:45 -1000 Subject: [PATCH 2/4] _convertPsToStep: no Microtone for whole numbers; one int() check _convertPsToStep returns None instead of Microtone(0) for whole numbers. The ps setter goes back to a single _convertPsToStep call rather than repeating the table lookup; Pitch.__init__ keeps an inline copy of the lookup, which saves a call (about 45 ns), with comments in both places to keep them in step. Whole numbers are found by comparing int(ps) with ps, which is cheaper than the isinstance check plus a second int(). TODOs note that _convertPsToStep should return an alter, so that the ps setter can change the existing accidental instead of replacing it. AI-assisted (Claude) --- music21/pitch.py | 70 +++++++++++++++++++++----------------- music21/test/test_pitch.py | 10 +++--- 2 files changed, 44 insertions(+), 36 deletions(-) diff --git a/music21/pitch.py b/music21/pitch.py index f88abf957..180b57c11 100644 --- a/music21/pitch.py +++ b/music21/pitch.py @@ -321,7 +321,7 @@ def _convertPsToStep( ps: int|float ) -> tuple[StepName, Accidental, - Microtone, + Microtone|None, int]: ''' Utility conversion; does not process internal representations. @@ -331,29 +331,31 @@ def _convertPsToStep( Returns a tuple of Step, an Accidental object, a Microtone object or None, and an int representing octave shift (which is nearly always zero, but can be 1 - for a B with very high microtones). + for a B with very high microtones). Whole numbers get None for the Microtone. >>> pitch._convertPsToStep(60) - ('C', , , 0) + ('C', , None, 0) + >>> pitch._convertPsToStep(60.0) + ('C', , None, 0) >>> pitch._convertPsToStep(66) - ('F', , , 0) + ('F', , None, 0) >>> pitch._convertPsToStep(67) - ('G', , , 0) + ('G', , None, 0) >>> pitch._convertPsToStep(68) - ('G', , , 0) + ('G', , None, 0) >>> pitch._convertPsToStep(-2) - ('B', , , 0) + ('B', , None, 0) >>> pitch._convertPsToStep(60.5) ('C', , , 0) >>> pitch._convertPsToStep(62) - ('D', , , 0) + ('D', , None, 0) >>> pitch._convertPsToStep(62.5) ('D', , , 0) >>> pitch._convertPsToStep(135) - ('E', , , 0) + ('E', , None, 0) >>> pitch._convertPsToStep(70) - ('B', , , 0) + ('B', , None, 0) >>> pitch._convertPsToStep(70.2) ('B', , , 0) >>> pitch._convertPsToStep(70.5) @@ -382,10 +384,17 @@ def _convertPsToStep( >>> pitch._convertPsToStep(59.9999999) ('C', , , 0) + + * Changed in v11: whole numbers return None instead of a Microtone of 0 cents. ''' - if isinstance(ps, int) or ps == int(ps): - step, wholeAlter = pitchClassToDefaultStepAlter[int(ps) % 12] - return step, Accidental(wholeAlter), Microtone(0), 0 + # TODO: make _convertPsToStep return an alter not an accidental, and then change + # the alter of the Pitch's existing accidental (setting displayStatus to None) + # to preserve style and displayType information + wholePs = int(ps) + if wholePs == ps: + # if this changes, update Pitch.__init__ too + step, wholeAlter = pitchClassToDefaultStepAlter[wholePs % 12] + return step, Accidental(wholeAlter), None, 0 else: # rounding here is essential ps = round(ps, PITCH_SPACE_SIG_DIGITS) @@ -1968,11 +1977,13 @@ def __init__(self, else: # is a number # is a midiNumber or a ps -- a float midiNumber # get step and accidental w/o octave - if isinstance(name, int) or name == int(name): - self._step, alter = pitchClassToDefaultStepAlter[int(name) % 12] + wholePs = int(name) + if wholePs == name: + # if this changes, update _convertPsToStep too + self._step, alter = pitchClassToDefaultStepAlter[wholePs % 12] self._accidental = Accidental(alter) else: - self.step, self._accidental = _convertPsToStep(name)[0:2] + self._step, self._accidental = _convertPsToStep(name)[0:2] self.spellingIsInferred = True if name >= 12: # is not a pitchClass self._octave = int(name / 12) - 1 @@ -2654,23 +2665,18 @@ def ps(self) -> float: @ps.setter def ps(self, value: int|float) -> None: - if isinstance(value, int) or value == int(value): - wholePs = int(value) - self._step, alter = pitchClassToDefaultStepAlter[wholePs % 12] - # a natural is None - self._accidental = Accidental(alter) if alter else None - self._microtone = None - self._octave = wholePs // 12 - 1 + # TODO: make _convertPsToStep return an alter not an accidental, and then change + # the alter of the Pitch's existing accidental (setting displayStatus to None) + # to preserve style and displayType information + # can assign microtone here; will be either None or a Microtone object + self._step, acc, self._microtone, octShift = _convertPsToStep(value) + + # replace a natural with a None + if acc.name == 'natural': + self._accidental = None else: - # can assign microtone here; will be either None or a Microtone object - self._step, acc, self._microtone, octShift = _convertPsToStep(value) - - # replace a natural with a None - if acc.name == 'natural': - self._accidental = None - else: - self._accidental = acc - self._octave = _convertPsToOct(value) + octShift + self._accidental = acc + self._octave = _convertPsToOct(value) + octShift # all ps settings must set implicit to True, as we do not know # what accidental this is diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index 92e9d62bd..8460e31ca 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -157,10 +157,12 @@ def testWholeNumberPsAndMidi(self): # Pitch(number) keeps the natural for midiNumber in range(12, 128): - p = Pitch(midiNumber) - self.assertEqual(p.nameWithOctave, f'{names[midiNumber % 12]}{midiNumber // 12 - 1}') - self.assertIsNotNone(p.accidental) - self.assertTrue(p.spellingIsInferred) + for value in (midiNumber, float(midiNumber)): + p = Pitch(value) + self.assertEqual(p.nameWithOctave, + f'{names[midiNumber % 12]}{midiNumber // 12 - 1}') + self.assertIsNotNone(p.accidental) + self.assertTrue(p.spellingIsInferred) From 8c5fe0ea07fa33ca594b4b6b02044bdff3225397 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Wed, 7 Oct 2026 20:00:38 -1000 Subject: [PATCH 3/4] testWholeNumberPsAndMidi: chosen cases, and a mock for the Note Test each pitch class once plus the octave edges (0, 11, 12, 127, and a negative ps) instead of every number from -13 to 139, and check that the Note is told once with a mock of pitchChanged instead of planting an entry in its private cache. AI-assisted (Claude) --- music21/test/test_pitch.py | 41 ++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index 8460e31ca..ff3802e1e 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -12,6 +12,7 @@ import copy import unittest +from unittest import mock from music21 import common from music21 import converter @@ -134,35 +135,37 @@ def testInitShortcutsMatchParsing(self): def testWholeNumberPsAndMidi(self): ''' Setting a whole-number .ps or .midi spells the pitch, resets the microtone, - marks the spelling inferred, and tells the Note. A natural is None. + marks the spelling inferred, and tells the Note once. A natural is None. ''' - names = ['C', 'C#', 'D', 'E-', 'E', 'F', 'F#', 'G', 'G#', 'A', 'B-', 'B'] - for ps in range(-13, 140): + # every pitch class once, then the octave edges and a negative ps + for ps, name, octave in [ + (60, 'C', 4), (61, 'C#', 4), (62, 'D', 4), (63, 'E-', 4), + (64, 'E', 4), (65, 'F', 4), (66, 'F#', 4), (67, 'G', 4), + (68, 'G#', 4), (69, 'A', 4), (70, 'B-', 4), (71, 'B', 4), + (0, 'C', -1), (11, 'B', -1), (12, 'C', 0), (127, 'G', 9), (-1, 'B', -2), + ]: for attribute, value in (('ps', ps), ('ps', float(ps)), ('midi', ps)): - if attribute == 'midi' and not 0 <= ps <= 127: - continue + if attribute == 'midi' and ps < 0: + continue # the midi setter wraps negative values into 0-11 with self.subTest(attribute=attribute, value=value): n = note.Note('D~5') n.pitch.microtone = 20 - n._cache['junk'] = 1 - setattr(n.pitch, attribute, value) - self.assertEqual(n.pitch.name, names[ps % 12]) - self.assertEqual(n.pitch.octave, ps // 12 - 1) - self.assertEqual(n.pitch.ps, ps) - if len(names[ps % 12]) == 1: + with mock.patch.object(n, 'pitchChanged') as pitchChanged: + setattr(n.pitch, attribute, value) + pitchChanged.assert_called_once() + self.assertEqual(n.pitch.name, name) + self.assertEqual(n.pitch.octave, octave) + if len(name) == 1: self.assertIsNone(n.pitch.accidental) self.assertEqual(n.pitch.microtone.cents, 0) self.assertTrue(n.pitch.spellingIsInferred) - self.assertEqual(n._cache, {}) # Pitch(number) keeps the natural - for midiNumber in range(12, 128): - for value in (midiNumber, float(midiNumber)): - p = Pitch(value) - self.assertEqual(p.nameWithOctave, - f'{names[midiNumber % 12]}{midiNumber // 12 - 1}') - self.assertIsNotNone(p.accidental) - self.assertTrue(p.spellingIsInferred) + for value, nameWithOctave in [(60, 'C4'), (60.0, 'C4'), (70, 'B-4'), (70.0, 'B-4')]: + p = Pitch(value) + self.assertEqual(p.nameWithOctave, nameWithOctave) + self.assertIsNotNone(p.accidental) + self.assertTrue(p.spellingIsInferred) From 43e4375886fa76c784d7e95b93aa267d257dbcb8 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Wed, 7 Oct 2026 20:07:37 -1000 Subject: [PATCH 4/4] Tests: only what is not covered elsewhere testPsAndMidiSettersInformNoteOnce checks that the ps and midi setters tell the Note once. testWholeNumberPsAndMidi keeps only cases the doctests do not cover: clearing a quarter-tone accidental and a microtone, a natural as None, a negative ps, the midi setter above 127, and Pitch(number) keeping its natural. AI-assisted (Claude) --- music21/test/test_pitch.py | 60 ++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/music21/test/test_pitch.py b/music21/test/test_pitch.py index fb1b933d5..e10b169f0 100644 --- a/music21/test/test_pitch.py +++ b/music21/test/test_pitch.py @@ -133,40 +133,44 @@ def testInitShortcutsMatchParsing(self): with self.assertRaises(PitchException): Pitch(step='CD') + def testPsAndMidiSettersInformNoteOnce(self): + for attribute in ('ps', 'midi'): + with self.subTest(attribute=attribute): + n = note.Note('D4') + with mock.patch.object(n, 'pitchChanged') as pitchChanged: + setattr(n.pitch, attribute, 61) + pitchChanged.assert_called_once() + def testWholeNumberPsAndMidi(self): ''' - Setting a whole-number .ps or .midi spells the pitch, resets the microtone, - marks the spelling inferred, and tells the Note once. A natural is None. + Setting a whole-number .ps or .midi replaces a quarter-tone accidental + and clears a microtone. ''' - # every pitch class once, then the octave edges and a negative ps - for ps, name, octave in [ - (60, 'C', 4), (61, 'C#', 4), (62, 'D', 4), (63, 'E-', 4), - (64, 'E', 4), (65, 'F', 4), (66, 'F#', 4), (67, 'G', 4), - (68, 'G#', 4), (69, 'A', 4), (70, 'B-', 4), (71, 'B', 4), - (0, 'C', -1), (11, 'B', -1), (12, 'C', 0), (127, 'G', 9), (-1, 'B', -2), - ]: - for attribute, value in (('ps', ps), ('ps', float(ps)), ('midi', ps)): - if attribute == 'midi' and ps < 0: - continue # the midi setter wraps negative values into 0-11 - with self.subTest(attribute=attribute, value=value): - n = note.Note('D~5') - n.pitch.microtone = 20 - with mock.patch.object(n, 'pitchChanged') as pitchChanged: - setattr(n.pitch, attribute, value) - pitchChanged.assert_called_once() - self.assertEqual(n.pitch.name, name) - self.assertEqual(n.pitch.octave, octave) - if len(name) == 1: - self.assertIsNone(n.pitch.accidental) - self.assertEqual(n.pitch.microtone.cents, 0) - self.assertTrue(n.pitch.spellingIsInferred) + p = Pitch('D~5') + p.microtone = 20 + p.ps = 61 + self.assertEqual(p.nameWithOctave, 'C#4') + self.assertEqual(p.microtone.cents, 0) + + # a natural is None, and a negative ps floors to the octave below + p.ps = 60.0 + self.assertEqual(p.nameWithOctave, 'C4') + self.assertIsNone(p.accidental) + p.ps = -1 + self.assertEqual((p.step, p.octave), ('B', -2)) + + # above 127, the midi setter wraps into the top octave; ps does not + p = Pitch() + p.midi = 130 + self.assertEqual(p.nameWithOctave, 'B-8') + p.ps = 130 + self.assertEqual(p.nameWithOctave, 'B-9') # Pitch(number) keeps the natural - for value, nameWithOctave in [(60, 'C4'), (60.0, 'C4'), (70, 'B-4'), (70.0, 'B-4')]: + for value in (60, 60.0): p = Pitch(value) - self.assertEqual(p.nameWithOctave, nameWithOctave) - self.assertIsNotNone(p.accidental) - self.assertTrue(p.spellingIsInferred) + self.assertEqual(p.nameWithOctave, 'C4') + self.assertEqual(p.accidental.name, 'natural')