Repository navigation
Faster Pitch(80) or p.midi = 20 or p.ps = 60 creation - #2071
Merged
Merged
Conversation
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)
mscuthbert
changed the title
mscuthbert
marked this pull request as ready for review
October 8, 2026 05:17
|
coverage: 93.371% (+0.002%) from 93.369% — pitch-midi-ps into master |
_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)
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)
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Faster whole-number
.ps,.midi, andPitch(number), part of the #2010 speedup work.New
pitchClassToDefaultStepAltergives the step and alter for each pitch class._convertPsToStepuses it for whole numbers and now returnsNonefor their Microtone instead ofMicrotone(0);Pitch(number)repeats the lookup inline to save a call. Thepssetter assigns step, accidental, and octave directly and informs the Note once instead of three times. Results are otherwise unchanged:p.midi = 60still gives no accidental andPitch(60)still gives a natural (#2010 will make them agree). A new test pins every whole-number ps and MIDI value; it passes on master too.p.midi = 60p.midi = 61p.ps = 61p.ps = 61.0p.ps = 61.5Pitch(60)Pitch(61)Pitch(midi=60)Note(60)Whole-file MIDI imports (
k525MIDIMvt1.mid,test03/04/09.mid, CANYON.MID) do not measurably change: pitches are a tiny part of them.AI-assisted (Claude)