-
Notifications
You must be signed in to change notification settings - Fork 441
(AI) Preserve microtones through Interval.transpose (fix accidental corruption / AccidentalException) #1962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3460,7 +3460,7 @@ def _diatonicTransposePitch(self, | |
| pitch1 = p | ||
| pitch2 = copy.deepcopy(pitch1) | ||
| oldDiatonicNum = pitch1.diatonicNoteNum | ||
| # centsOrigin = pitch1.microtone.cents # unused!! | ||
| centsOrigin = pitch1.microtone.cents | ||
| distanceToMove = self.diatonic.generic.staffDistance | ||
|
|
||
| newDiatonicNumber = oldDiatonicNum + distanceToMove | ||
|
|
@@ -3475,8 +3475,14 @@ def _diatonicTransposePitch(self, | |
|
|
||
| # We have the right note name but not the right accidental | ||
| interval2 = Interval(pitch1, pitch2) | ||
| # halfStepsToFix already has any microtones | ||
| # interval2 is measured from the microtonal pitch1, so halfStepsToFix | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pitch1 isn't necessarily microtonal. This comments should be trimmed to essentials and moved into the |
||
| # carries the source microtone. Peel it off so it adjusts only the | ||
| # accidental; it is restored as a microtone once the accidental is set. | ||
| # Without this, the leaked cents corrupt the spelling (e.g. a +30c pitch | ||
| # transposed by P1 becomes a half-sharp) or raise AccidentalException. | ||
| halfStepsToFix = self.chromatic.semitones - interval2.chromatic.semitones | ||
| if centsOrigin: | ||
| halfStepsToFix = round(halfStepsToFix - centsOrigin / 100.0, 9) | ||
|
|
||
| # environLocal.printDebug(['self', self, 'halfStepsToFix', halfStepsToFix, | ||
| # 'centsOrigin', centsOrigin, 'interval2', interval2]) | ||
|
|
@@ -3527,6 +3533,10 @@ def _diatonicTransposePitch(self, | |
| and oldPitch2Accidental.name == 'natural'): | ||
| pitch2.accidental = oldPitch2Accidental | ||
|
|
||
| # restore the source microtone that was peeled off halfStepsToFix above | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this does not restore the source microtone, it restores the microtone's |
||
| if centsOrigin: | ||
| pitch2.microtone = pitch2.microtone.cents + centsOrigin | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pitch2.microtone.cents = ... |
||
|
|
||
| if useImplicitOctave: | ||
| pitch2.octave = None | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,6 +350,51 @@ def testIntervalMicrotonesB(self): | |
| i = interval.Interval(note.Note('c4'), note.Note('c~4')) | ||
| self.assertEqual(str(i), '<music21.interval.Interval A1 (-50c)>') | ||
|
|
||
| def testTransposeMicrotonePreserved(self): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good test |
||
| # A microtone (cents offset) must survive transposition unchanged and | ||
| # must not be turned into a spurious quarter-tone accidental. Formerly | ||
| # the source microtone leaked into the accidental computation, so e.g. | ||
| # C4(+30c) transposed by a unison became C~4(-20c), or larger microtones | ||
| # raised AccidentalException. | ||
|
|
||
| # Transposing by a perfect unison is the identity. | ||
| p = pitch.Pitch('C4') | ||
| p.microtone = 30 | ||
| out = p.transpose(interval.Interval('P1')) | ||
| self.assertEqual(out.nameWithOctave, 'C4') | ||
| self.assertEqual(out.microtone.cents, 30) | ||
|
|
||
| # A formerly-crashing case: G#5(+50c) transposed by an augmented unison. | ||
| p = pitch.Pitch('G#5') | ||
| p.microtone = 50 | ||
| out = p.transpose(interval.Interval('A1')) | ||
| self.assertEqual(out.nameWithOctave, 'G##5') | ||
| self.assertEqual(out.microtone.cents, 50) | ||
|
|
||
| # A quarter-tone accidental and a microtone both survive a unison. | ||
| p = pitch.Pitch('C~4') | ||
| p.microtone = 30 | ||
| out = p.transpose(interval.Interval('P1')) | ||
| self.assertEqual(out.name, 'C~') | ||
| self.assertEqual(out.microtone.cents, 30) | ||
|
|
||
| # Across intervals and microtone magnitudes: the cents are preserved, | ||
| # the sounding pitch moves by exactly the interval, and no quarter-tone | ||
| # accidental is introduced on an integer-semitone interval. | ||
| for iName in ['P1', 'm2', 'M3', 'P5', 'm7', 'P8', 'A4', 'd5']: | ||
| iv = interval.Interval(iName) | ||
| for cents in (-49, -25, 26, 49): | ||
| with self.subTest(interval=iName, cents=cents): | ||
| src = pitch.Pitch('C4') | ||
| src.microtone = cents | ||
| psBefore = src.ps | ||
| out = src.transpose(iv) | ||
| self.assertAlmostEqual(out.microtone.cents, cents) | ||
| self.assertAlmostEqual(out.ps - psBefore, | ||
| iv.chromatic.semitones) | ||
| if out.accidental is not None: | ||
| self.assertEqual(out.accidental.alter % 1, 0.0) | ||
|
|
||
| def testDescendingAugmentedUnison(self): | ||
| ns = note.Note('C4') | ||
| ne = note.Note('C-4') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should gate on
if not pitch1.isTwelveTone():...else: centsOrigin = 0.0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a patch that should make
isTwelveTonerun faster -- will push in next PR.isTwelveToneguards against creating Microtone objects unnecessarily.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #1963