From 1aec1158c20d8de7644e3d33896d9ce20a71b079 Mon Sep 17 00:00:00 2001 From: TimFelix <35711942+TimFelixBeyer@users.noreply.github.com> Date: Fri, 1 Mar 2024 00:31:45 +0100 Subject: [PATCH 1/3] Correct scale caching mechanism. 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). --- music21/roman.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/music21/roman.py b/music21/roman.py index e53c1ad07..d8c8de76f 100644 --- a/music21/roman.py +++ b/music21/roman.py @@ -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) @@ -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 ' @@ -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 From 764f5097b0b192db5191be2d0f410b81c782cf8a Mon Sep 17 00:00:00 2001 From: TimFelix <35711942+TimFelixBeyer@users.noreply.github.com> Date: Fri, 1 Mar 2024 00:36:58 +0100 Subject: [PATCH 2/3] Include scale name in hash again --- music21/roman.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/music21/roman.py b/music21/roman.py index d8c8de76f..007aba86c 100644 --- a/music21/roman.py +++ b/music21/roman.py @@ -3489,10 +3489,11 @@ def key(self, keyOrScale): elif 'Scale' in keyClasses: # Need unique 'hash' for each scale. Names is not enough as there are # multiple scales with the same name but different pitches. + scaleHash = keyOrScale.name if getattr(keyOrScale, 'isConcrete', False): - scaleHash = "_".join([p.nameWithOctave for p in keyOrScale.pitches]) - else: - scaleHash = keyOrScale.name + scaleHash += "_".join( + [p.nameWithOctave for p in keyOrScale.pitches] + ) if scaleHash in _scaleCache: keyOrScale = _scaleCache[scaleHash] else: From 207d3a56543b8fa59922df4988a2a2df282d69b3 Mon Sep 17 00:00:00 2001 From: TimFelix <35711942+TimFelixBeyer@users.noreply.github.com> Date: Fri, 1 Mar 2024 00:39:27 +0100 Subject: [PATCH 3/3] fix lint --- music21/roman.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/music21/roman.py b/music21/roman.py index 007aba86c..c810c98f9 100644 --- a/music21/roman.py +++ b/music21/roman.py @@ -3491,7 +3491,7 @@ def key(self, keyOrScale): # multiple scales with the same name but different pitches. scaleHash = keyOrScale.name if getattr(keyOrScale, 'isConcrete', False): - scaleHash += "_".join( + scaleHash += '_'.join( [p.nameWithOctave for p in keyOrScale.pitches] ) if scaleHash in _scaleCache: