Skip to content

Commit

Permalink
Correct scale caching mechanism.
Browse files Browse the repository at this point in the history
We cannot use the name of a ConcreteScale to cache the scales as there are sometimes different scales with the same name.
Using the pitch list and names now to be more precise and avoid collisions (#1653).
  • Loading branch information
TimFelixBeyer committed Feb 29, 2024
1 parent 0927e39 commit 1aec115
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions music21/roman.py
Original file line number Diff line number Diff line change
Expand Up @@ -2488,10 +2488,10 @@ def _parseFigure(self):
if not isinstance(self._figure, str): # pragma: no cover
raise RomanNumeralException(f'got a non-string figure: {self._figure!r}')

if not self.useImpliedScale:
useScale = self._scale
else:
if self.useImpliedScale:
useScale = self.impliedScale
else:
useScale = self._scale

(workingFigure, useScale) = self._correctForSecondaryRomanNumeral(useScale)

Expand Down Expand Up @@ -3487,10 +3487,16 @@ def key(self, keyOrScale):
# store for later
_keyCache[keyOrScale.tonicPitchNameWithCase] = keyOrScale
elif 'Scale' in keyClasses:
if keyOrScale.name in _scaleCache:
keyOrScale = _scaleCache[keyOrScale.name]
# Need unique 'hash' for each scale. Names is not enough as there are
# multiple scales with the same name but different pitches.
if getattr(keyOrScale, 'isConcrete', False):
scaleHash = "_".join([p.nameWithOctave for p in keyOrScale.pitches])
else:
scaleHash = keyOrScale.name
if scaleHash in _scaleCache:
keyOrScale = _scaleCache[scaleHash]
else:
_scaleCache[keyOrScale.name] = keyOrScale
_scaleCache[scaleHash] = keyOrScale
else:
raise RomanNumeralException(
f'Cannot get a key from this object {keyOrScale!r}, send only '
Expand Down Expand Up @@ -4553,6 +4559,14 @@ def test_int_figure_case_matters(self):
rn = RomanNumeral(4, 'C')
self.assertEqual(rn.figure, 'IV')

def test_scale_caching(self):
mcs = scale.ConcreteScale('c', pitches=('C', 'D', 'E', 'F', 'G', 'A', 'B'))
rn = mcs.romanNumeral('IV7')
self.assertEqual([p.unicodeName for p in rn.pitches], ['F', 'A', 'C', 'E'])
mcs = scale.ConcreteScale('c', pitches=('C', 'D', 'E-', 'F', 'G', 'A', 'B'))
rn = mcs.romanNumeral('IV7')
self.assertEqual([p.unicodeName for p in rn.pitches], ['F', 'A', 'C', 'E♭'])


class TestExternal(unittest.TestCase):
show = True
Expand Down

0 comments on commit 1aec115

Please sign in to comment.