Fix a regression with chord accidentals

Better solution than cuthbertLab#1299

music21/pitch.py

@@ -4733,8 +4733,10 @@ def _stepInKeySignature(self, alteredPitches):
 
     def updateAccidentalDisplay(
         self,
-        pitchPast: t.Optional[t.List['Pitch']] = None,
+        *,
+        pitchPast: t.Optional[t.List['Pitch', t.List['Pitch']]] = None,
         pitchPastMeasure: t.Optional[t.List['Pitch']] = None,
+        otherSimultaneousPitches: t.Optional[t.List['Pitch']] = None,
         alteredPitches: t.Optional[t.List['Pitch']] = None,
         cautionaryPitchClass: bool = True,
         cautionaryAll: bool = False,
@@ -4784,13 +4786,13 @@ def updateAccidentalDisplay(
 
         >>> a = pitch.Pitch('a')
         >>> past = [pitch.Pitch('a#'), pitch.Pitch('c#'), pitch.Pitch('c')]
-        >>> a.updateAccidentalDisplay(past, cautionaryAll=True)
+        >>> a.updateAccidentalDisplay(pitchPast=past, cautionaryAll=True)
         >>> a.accidental, a.accidental.displayStatus
         (<music21.pitch.Accidental natural>, True)
 
         >>> b = pitch.Pitch('a')
         >>> past = [pitch.Pitch('a#'), pitch.Pitch('c#'), pitch.Pitch('c')]
-        >>> b.updateAccidentalDisplay(past)  # should add a natural
+        >>> b.updateAccidentalDisplay(pitchPast=past)  # should add a natural
         >>> b.accidental, b.accidental.displayStatus
         (<music21.pitch.Accidental natural>, True)
 
@@ -4799,10 +4801,11 @@ def updateAccidentalDisplay(
 
         >>> a4 = pitch.Pitch('a4')
         >>> past = [pitch.Pitch('a#3'), pitch.Pitch('c#'), pitch.Pitch('c')]
-        >>> a4.updateAccidentalDisplay(past, cautionaryPitchClass=False)
+        >>> a4.updateAccidentalDisplay(pitchPast=past, cautionaryPitchClass=False)
         >>> a4.accidental is None
         True
 
+        v8 -- made keyword-only and added `otherSimultaneousPitches`.
         '''
         # N.B. -- this is a very complex method
         # do not alter it without significant testing.
@@ -4827,10 +4830,6 @@ def set_displayStatus(newDisplayStatus: bool):
         if alteredPitches is None:
             alteredPitches = []
 
-        # TODO: this presently deals with chords as simply a list
-        #    we might permit pitchPast to contain a list of pitches, to represent
-        #    a simultaneity?
-
         # should we display accidental if no previous accidentals have been displayed
         # i.e. if it's the first instance of an accidental after a tie
         displayAccidentalIfNoPreviousAccidentals = False
@@ -4861,6 +4860,16 @@ def set_displayStatus(newDisplayStatus: bool):
             else:
                 return  # exit: nothing more to do
 
+        if (otherSimultaneousPitches
+            and any(
+                pSimult.step == self.step
+                and (cautionaryPitchClass or self.octave is pSimult.octave != self.octave)
+                    for pSimult in otherSimultaneousPitches
+                )
+        ):
+            set_displayStatus(True)
+            return
+
         # no pitches in past...
         if not pitchPastAll:
             # if we have no past, we show the accidental if this pitch name
@@ -5061,13 +5070,6 @@ def set_displayStatus(newDisplayStatus: bool):
                       and pPastInMeasure is False):
                     set_displayStatus(True)
 
-                # Avoid making early, incorrect assumptions if this pitch is part of a chord.
-                # pPast might include this note itself, in which case we should not say
-                # "natural already in past usage". (Filtering out the current note from pPast
-                # when calling this method is not sufficient, because there could be repetitions.)
-                elif self._client is not None and self._client._chordAttached is not None:
-                    continue
-
                 # other cases: already natural in past usage, do not need
                 # natural again (and not in key sig)
                 else:

music21/stream/base.py

@@ -6319,6 +6319,7 @@ def makeAccidentals(
         *,
         pitchPast: t.Optional[t.List[pitch.Pitch]] = None,
         pitchPastMeasure: t.Optional[t.List[pitch.Pitch]] = None,
+        otherSimultaneousPitches: t.Optional[t.List[pitch.Pitch]] = None,
         useKeySignature: t.Union[bool, key.KeySignature] = True,
         alteredPitches: t.Optional[t.List[pitch.Pitch]] = None,
         searchKeySignatureByContext: bool = False,
@@ -6336,6 +6337,7 @@ def makeAccidentals(
 
         `pitchPastMeasure` is a list of pitches preceding this pitch but in a previous measure.
 
+        `otherSimultaneousPitches` is a list of other pitches in this simultaneity.
 
         If `useKeySignature` is True, a :class:`~music21.key.KeySignature` will be searched
         for in this Stream or this Stream's defined contexts. An alternative KeySignature
@@ -6377,7 +6379,7 @@ def makeAccidentals(
 
         Changed in v.6: does not return anything if inPlace is True.
         Changed in v.7: default inPlace is False
-        Changed in v.8: altered unisons in Chords now supply clarifying naturals.
+        Changed in v.8: altered unisons/octaves in Chords now supply clarifying naturals.
 
         All arguments are keyword only.
         '''
@@ -6459,11 +6461,9 @@ def makeAccidentals(
                     tiePitchSet.add(e.pitch.nameWithOctave)
 
             elif isinstance(e, chord.Chord):
-                # add all chord elements to past first
                 # when reading a chord, this will apply an accidental
                 # if pitches in the chord suggest an accidental
                 seenPitchNames = set()
-                pitchPast += e.pitches
 
                 for n in list(e):
                     p = n.pitch
@@ -6472,9 +6472,12 @@ def makeAccidentals(
                     else:
                         lastNoteWasTied = False
 
+                    otherSimultaneousPitches = [other_p for other_p in e.pitches if other_p is not p]
+
                     p.updateAccidentalDisplay(
                         pitchPast=pitchPast,
                         pitchPastMeasure=pitchPastMeasure,
+                        otherSimultaneousPitches=otherSimultaneousPitches,
                         alteredPitches=alteredPitches,
                         cautionaryPitchClass=cautionaryPitchClass,
                         cautionaryAll=cautionaryAll,
@@ -6489,6 +6492,7 @@ def makeAccidentals(
                 for pName in seenPitchNames:
                     tiePitchSet.add(pName)
 
+                pitchPast += e.pitches
             else:
                 tiePitchSet.clear()
 

music21/stream/tests.py

@@ -2613,10 +2613,8 @@ def testMakeAccidentalsA(self):
         self.assertTrue(c1.pitches[1].accidental.displayStatus)
         self.assertTrue(c1.pitches[2].accidental.displayStatus)
 
-        # not getting a natural here because of chord tones
-        # self.assertTrue(n3.pitch.accidental.displayStatus)
-        # self.assertEqual(n3.pitch.accidental, None)
-        # s.show()
+        # not necessary to repeat the natural afterward
+        self.assertIsNone(n3.pitch.accidental, None)
 
         s = Stream()
         n1 = note.Note('a#')
@@ -2794,6 +2792,29 @@ def testMakeAccidentalsRespectsDisplayType(self):
 
         # TODO: other types
 
+    def testMakeAccidentalsOnChord(self):
+        c = chord.Chord('F# A# C#')
+        s = Stream(c)
+        self.assertFalse(any(n.pitch.accidental.displayStatus for n in c))
+        s.makeAccidentals(inPlace=True)
+        self.assertTrue(all(n.pitch.accidental.displayStatus for n in c))
+
+        augmented_octave = chord.Chord('F4 F#5')
+        s2 = Stream(augmented_octave)
+        self.assertIsNone(augmented_octave[0].pitch.accidental)
+        s2.makeAccidentals(inPlace=True)
+        self.assertTrue(augmented_octave[0].pitch.accidental.displayStatus)
+
+        # Repeat the test without octaves and reset state
+        low, high = augmented_octave.pitches
+        low.octave = None
+        low.accidental = None
+        high.octave = None
+        high.accidental.displayStatus = None
+
+        s2.makeAccidentals(inPlace=True)
+        self.assertTrue(all(n.pitch.accidental.displayStatus for n in augmented_octave))
+
     def testMakeNotationTiesKeyless(self):
         p = converter.parse('tinynotation: 4/4 f#1~ f#1')
         # Key of no sharps/flats