Repository navigation
Add support for implicit octaves - #335
Merged
Merged
Conversation
Ports the music21p `octave-int` branch (7a81a6bc0..8c65af725). m21j was
already halfway there: `_octave` was seeded to 4 so `.octave` was in
practice always a number, but `implicitOctave`, `unicodeNameWithOctave`
and `_getEnharmonicHelper` still tested for `undefined`, and
`ChromaticInterval.transposePitch` set `.octave = undefined` on a
"not yet implemented in m21j" path -- which would have crashed
`nameWithOctave` (`undefined.toString()`). So the octaveless concept
existed only as dead branches.
Now `_octave` is undefined until an octave is given; `.octave` reports
`defaults.pitchOctave` (new, 4) in that case and `.octaveIsImplicit`
tells the two apart. `implicitOctave` becomes a deprecated synonym for
`.octave`. Implicitness survives cloning, transposition and enharmonic
respelling, as in m21p.
Behavior changes, all matching m21p:
- `new Pitch('C').nameWithOctave` is 'C', not 'C4' (likewise
`unicodeNameWithOctave`, `toString()`, `Note.nameWithOctave`).
- `new Pitch('C').eq(new Pitch('C4'))` is false; `eq` compares the
stored octave, not the reported one.
- `KeySignature.alteredPitches` and `Key.tonic` are octaveless.
- `Pitch(3)` (a pitchClass) is octaveless; `Pitch(65)` (midi) is not.
- `nameWithOctave = 'C'` sets octaveIsImplicit instead of throwing on a
null regex match.
Places that need a real octave pin one down rather than inheriting the
default, again as m21p's intervalNetwork does: `AbstractScale`'s
`getRealization` and `getPitchFromNodeDegree` (so `Key('E-')`, whose
tonic is octaveless, still realizes E-4..E-5 and `pitchFromDegree(5)`
still climbs), and `Interval.transposePitch`, which transposes with an
octave and forgets it afterwards so intervals wider than an octave
spell correctly.
MusicXML export reads `.octave` directly now that `implicitOctave` is
just a synonym.
AI-assisted (Claude)
music21p keeps `octave = None` only as a deprecation shim (removed in v13); m21j has no such history to preserve, so the setter takes a plain number. `octaveIsImplicit = true` is the way to make a Pitch octaveless. AI-assisted (Claude)
The three interval transposePitch methods and _getEnharmonicHelper build a separate pitch, so the source flag is still valid at the end; one extra dereference is cheaper than the local. simplifyEnharmonic keeps its local: returnObj is `this` when inPlace, so setting .ps clears the flag before it would be read. AI-assisted (Claude)
An implicit octave reports defaults.pitchOctave, which is the same anchor `pKeep.octave = 4` supplied, so the loop can hand out the transposed pitch itself instead of an explicit copy. 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
1 participant
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.
Music21p started off with a very broad concept of implicit octaves -- simply make octave = None and tada -- it was pitch without octave. But sometimes you need an octave, so it had
.implicitOctavewhich gave4for octaveless pitches.By the time I started on music21j I recognized that music21p's version was overly complicated. So music21j only had octave as a number (int). It never supported implicit octaves.
Now as of cuthbertLab/music21#2023 music21p also makes octave only a number. Here we add "octaveIsImplicit" for parity with music21p and leave our always-int
.octavealone.Other changes to sync w/ music21p but break older behavior here:
AI-assisted (Claude)