diff --git a/music21/chord/__init__.py b/music21/chord/__init__.py index a0d6789f6..83977eae3 100644 --- a/music21/chord/__init__.py +++ b/music21/chord/__init__.py @@ -964,17 +964,26 @@ def _removePitchByRedundantAttribute( ''' Common method for stripping pitches based on redundancy of one pitch attribute. The `attribute` is provided by a string. + + attribute `'nameWithOctave'` uses a special case to make comparisons with + negative octaves work properly ''' if not inPlace: # make a copy returnObj = copy.deepcopy(self) else: returnObj = self - uniquePitches = [] + uniquePitches: set[t.Any] = set() deleteComponents = [] for comp in returnObj._notes: - if getattr(comp.pitch, attribute) not in uniquePitches: - uniquePitches.append(getattr(comp.pitch, attribute)) + if attribute == 'nameWithOctave': + # for comparing with negative octaves etc. + value = (comp.pitch.name, comp.pitch.octave, comp.pitch.octaveIsImplicit) + else: + value = getattr(comp.pitch, attribute) + + if value not in uniquePitches: + uniquePitches.add(value) else: deleteComponents.append(comp) @@ -3615,41 +3624,37 @@ def removeRedundantPitches(self, *, inPlace: bool = False) -> t.Self|list[pitch. >>> c2c - It is a known bug that because pitch.nameWithOctave gives - the same value for B-flat in octave 1 as B-natural in octave - negative 1, negative octaves can screw up this method. - With all the things left to do for music21, it doesn't seem - a bug worth squashing at this moment, but FYI: + Pitches with extreme octaves (whose `.nameWithOctave`s + look identical) are still distinguished. + + >>> p1 = pitch.Pitch('B') + >>> p1.octave = -1 + >>> p2 = pitch.Pitch('B-') + >>> p2.octave = 1 + >>> p1.nameWithOctave == p2.nameWithOctave + True - >>> p1 = pitch.Pitch('B-') - >>> p1.octave = 1 - >>> p2 = pitch.Pitch('B') - >>> p2.octave = -1 >>> c3 = chord.Chord([p1, p2]) - >>> removedPitches = c3.removeRedundantPitches(inPlace=True) - >>> c3.pitches - (,) + >>> c3 + - >>> c3.pitches[0].name - 'B-' - >>> c3.pitches[0].octave - 1 + >>> removedPitches = c3.removeRedundantPitches(inPlace=True) >>> removedPitches - [] - >>> removedPitches[0].name - 'B' - >>> removedPitches[0].octave - -1 - - The first pitch survives: + [] + >>> c3 + + >>> [(p.name, p.octave) for p in c3.pitches] + [('B', -1), ('B-', 1)] - >>> c3.pitches[0] is p1 - True + A pitch with no octave of its own is likewise not the same pitch as + one placed in the default octave: - >>> c3.pitches[0] is p2 - False + >>> c4 = chord.Chord([pitch.Pitch('C'), pitch.Pitch('C4')]) + >>> c4.removeRedundantPitches() + * Changed in v6: inPlace defaults to False. + * Changed in v11: works properly with negative octaves ''' return self._removePitchByRedundantAttribute('nameWithOctave', inPlace=inPlace) diff --git a/music21/test/test_chord.py b/music21/test/test_chord.py index caa50790e..25f0e94d0 100644 --- a/music21/test/test_chord.py +++ b/music21/test/test_chord.py @@ -739,6 +739,34 @@ def testChordCannotContainUnpitched(self): # noinspection PyTypeChecker Chord([note.Unpitched()]) # type: ignore + def testRemoveRedundantPitchesTellsNegativeOctavesFromFlats(self): + ''' + nameWithOctave spells both B-flat in octave 1 and B-natural in + octave -1 as 'B-1', so comparing pitches by that string made the + second look like a repeat of the first. + ''' + flat = pitch.Pitch('B-') + flat.octave = 1 + natural = pitch.Pitch('B') + natural.octave = -1 + self.assertEqual(flat.nameWithOctave, natural.nameWithOctave) + + ch = Chord([flat, natural]) + self.assertEqual(ch.removeRedundantPitches(inPlace=True), []) + self.assertEqual([(p.name, p.octave) for p in ch.pitches], + [('B-', 1), ('B', -1)]) + + # a real repeat still goes, whatever octave it is in + repeated = Chord([flat, natural, pitch.Pitch('B-1')]) + removed = repeated.removeRedundantPitches(inPlace=True) + self.assertEqual([p.nameWithOctave for p in removed], ['B-1']) + self.assertEqual(len(repeated.pitches), 2) + + # and a pitch with no octave is not the one in the default octave + octaveless = Chord([pitch.Pitch('C'), pitch.Pitch('C4')]) + self.assertEqual(octaveless.removeRedundantPitches(inPlace=True), []) + self.assertEqual(len(octaveless.pitches), 2) + def testCacheClearedOnAdd(self): ch = chord.Chord('C4 E4 G4') self.assertTrue(ch.isConsonant())