cuthbertLab / music21

music21 is a Toolkit for Computational Musicology
https://www.music21.org/
Other
2.12k stars 401 forks source link

overridden roots should be ignored when creating a romanNumeral from a ChordSymbol. #420

Open mscuthbert opened 5 years ago

mscuthbert commented 5 years ago

From Beach Clark:

>>> cs = harmony.ChordSymbol('C6')
>>> cs.writeAsChord = False
>>> cs.key = key.Key('A')
>>> tonic_note = note.Note(cs.key.tonicPitchNameWithCase)
>>> rn = cs.romanNumeral
>>> rn
<music21.roman.RomanNumeral vi43 in A major>

overridden roots should be ignored when creating a romanNumeral from a ChordSymbol.

mscuthbert commented 5 years ago

actually a problem with writeAsChord calling updatePitches.

jacobtylerwalls commented 4 years ago

FYI I observe the following after the change in #604:

>>> chord = harmony.ChordSymbol('C6')
>>> chord.key = key.Key('A')
>>> rn = chord.romanNumeral
>>> rn
<music21.roman.RomanNumeral bIII#65#3 in A major>

The behavior is the same whether or not chord.writeAsChord = False.

That looks mostly correct--save for the alterations over C being a little screwy, probably on account of the nondiatonic root--but are you suggesting we should disregard C as the root and build I65 in A major?

mscuthbert commented 4 years ago

Ah, that does look better -- one more reason to merge #605. And the alteration problems here are separate from the ChordSymbol problem, but come from the overridden root:

In [2]: harmony.ChordSymbol('C6').pitches                                                                                                                                     
Out[2]: 
(<music21.pitch.Pitch C3>,
 <music21.pitch.Pitch E3>,
 <music21.pitch.Pitch G3>,
 <music21.pitch.Pitch A3>)

In [3]: c = chord.Chord('C3 E3 G3 A3')                                                                                                                                        

In [4]: roman.romanNumeralFromChord(c, 'A')                                                                                                                                   
Out[4]: <music21.roman.RomanNumeral i6b5 in A major>

In [5]: c.root(c.bass())                                                                                                                                                      

In [6]: roman.romanNumeralFromChord(c, 'A')                                                                                                                                  
Out[6]: <music21.roman.RomanNumeral bIII#65#3 in A major>
mscuthbert commented 3 years ago

Still gives problems after #605 was merged.

jacobtylerwalls commented 3 years ago

It's ugly for sure. What is the result you expect? I can't think of anything better offhand.

jacobtylerwalls commented 3 years ago

I suppose bIII65 would be fine. 65 are the figures above the bass, and bIII is the root, and no accidentals are necessary in this key. It breaks the usual ability to backsolve for the bass note from the roman and the figures, but in this scenario I don't think backsolving for the bass is a reasonable request; the bass is known.

jacobtylerwalls commented 3 years ago

Er, bIII6b5 that is--need to get the Gn.

This diff seems to work/pass the test suite. @MarkGotham does it break anything in your projects? I'm worried there a lot of figures and augmented sixths that would need to be tested. I'm not sure why this branch I'm skipping was originally desired. (Edited the diff to make it easier to read.)

diff --git a/music21/roman.py b/music21/roman.py
index b67d08503..95f27c9ee 100644
--- a/music21/roman.py
+++ b/music21/roman.py
@@ -658,7 +658,7 @@ def romanNumeralFromChord(
     ...     key.Key('g#'),
     ...     )
     >>> rn
-    <music21.roman.RomanNumeral bivo6 in g# minor>
+    <music21.roman.RomanNumeral bivob63 in g# minor>

     The pitches remain the same with the same octaves:

@@ -778,7 +778,7 @@ def romanNumeralFromChord(
     ...     chord.Chord('E-4 G4 C#5'),
     ...     key.Key('C')
     ...     )
-    <music21.roman.RomanNumeral #io6b3 in C major>
+    <music21.roman.RomanNumeral #io#63 in C major>

     Changed in v7 -- i7 is given for a tonic or subdominant minor-seventh chord in major:
@@ -856,7 +856,7 @@ def romanNumeralFromChord(
     # <music21.roman.RomanNumeral IV#75#3 in c minor>
     '''
     aug6subs = {
-        '#ivo6b3': 'It6',
+        '#ivo#63': 'It6',
         'IIø#643': 'Fr43',
         '#ii64b3': 'Sw43',
         '#ivo6bb5b3': 'Ger65',
@@ -912,22 +912,10 @@ def romanNumeralFromChord(
     ft = figureTupleSolo(root, keyObj, keyObj.tonic)  # a FigureTuple
     ft = correctRNAlterationForMinor(ft, keyObj)

-    if ft.alter == 0:
-        tonicPitch = keyObj.tonic
-    else:
-        # Altered scale degrees, such as #V require a different hypothetical
-        # tonic:
-
-        # not worth caching yet -- 150 microseconds; we're trying to lower milliseconds
-        transposeInterval = interval.intervalFromGenericAndChromatic(
-            interval.GenericInterval(1),
-            interval.ChromaticInterval(ft.alter))
-        tonicPitch = transposeInterval.transposePitch(keyObj.tonic)
-
     if keyObj.mode == 'major':
-        tonicPitchName = tonicPitch.name.upper()
+        tonicPitchName = keyObj.tonic.name.upper()
     else:
-        tonicPitchName = tonicPitch.name.lower()
+        tonicPitchName = keyObj.tonic.name.lower()

     alteredKeyObj = _getKeyFromCache(tonicPitchName)

@@ -3761,6 +3749,12 @@ class Test(unittest.TestCase):
         rn = RomanNumeral('vii[no5]', 'a')
         self.assertEqual([p.name for p in rn.pitches], ['G#', 'B'])

+    def testRootOverrideChromaticContext(self):
+        cs = chord.Chord(['C4', 'E4', 'G4', 'A4'])
+        cs.root(cs.bass())
+        rn = romanNumeralFromChord(cs, 'A')
+        self.assertEqual(rn.figure, 'bIII6b5')
+
     def testNeapolitan(self):
         # False:
         rn = RomanNumeral('III', 'a')  # Not II
MarkGotham commented 3 years ago

Hi all. Thanks for the invitation to the party @jacobtylerwalls and for the reminder just now: please excuse the delayed reaction!

Here's my 2 cents:

For better or worse(!), I'd think the Roman numerals default would be to handle the chord C3 E3 G3 A3 as an A minor seventh. That can be taken as a criticism of the Roman numeral approach in general, but that criticism would be a separate question from how to implement it.

Equivalently, anything of the form X65 is a 7th chord with X as the root (sic), in first inversion (so the root's not in bass).

So then in cases where we want to insist on a Roman numeral for C3 E3 G3 A3 as a C major chord with an added sixth (hello Rameau!) we would need an [add6]. That's a more cumbersome notation, but then again that inconvenience speaks to the divergence from normal practice and we should expect a certain amount of this in converting between contexts with such divergent norms as RNs and chord symbols.

As for RNs like bIII#65#3 this is still a big problem for romanNumeralFromChord. At the least, #3 has got to be rare given that any upper case Roman numeral has already specified the third quality as major. There's a lots of cases where we need augmented sixths (and so diminished thirds) and a decent number of diminished 4ths (hello Purcell!), but augmented thirds from the root?

jacobtylerwalls commented 3 years ago

So for this:

>>> c = chord.Chord('C3 E3 G3 A3')  
>>> c.root(c.pitches[0])
>>> roman.romanNumeralFromChord(c, 'A')

We now have these options on the menu:

<music21.roman.RomanNumeral bIII#65#3 in A major>  # current, wrong
<music21.roman.RomanNumeral bIII6b5 in A major>  # LGTM, but breaks what Mark calls "equivalence" or what I called the ability to "backsolve" the root from the bass and figures
<music21.roman.RomanNumeral bIII[add6] in A major>  # Mark's proposal
<music21.roman.RomanNumeral bIIIb5[add6] in A major>  # more precise version
<music21.roman.RomanNumeral i65 in A major>  # disregard the user's root
--something else?--

Mark's proposal could work, but I would be concerned about the amount of effort going into building a kind of Frankenstein RomanNumeralChordSymbol that mixes the two kinds of notation. In fact, option 4, what I labeled "more precise version"--I think shows we need the above diff I posted anyway, even if we eventually take up Mark's proposal.

Equivalently, anything of the form X65 is a 7th chord with X as the root (sic), in first inversion (so the root's not in bass).

I don't mind breaking equivalence (I suppose that's why I always appreciated V64 for cad64). It's nice when the figures and bass let you backsolve for the root, but when they don't, tough luck--if the root is known (i.e. overridden by the user), no use backsolving for it. I also don't like option no. 5, just ignoring what the user specified and saying, "nope, the root is still A." But I'm repeating myself.

As for RNs like bIII#65#3 this is still a big problem for romanNumeralFromChord.

Can you let me know more about this--are there other problems this example doesn't target?

MarkGotham does it break anything in your projects? I'm worried there a lot of figures and augmented sixths that would need to be tested. I'm not sure why this branch I'm skipping was originally desired.

I'm still kinda curious if this diff breaks any assumptions at When-in-Rome and its corpus. That would be useful information.

jacobtylerwalls commented 3 years ago

I think we've got two separate issues here. I reconsidered--Mark's right, this should work:

# from doctest
>>> rn = roman.RomanNumeral('V65[no5][add#6][b3]', key.Key('C'))
>>> rn.addedSteps
[('#', 6)]
>>> c = harmony.ChordSymbol('C6')
>>> rn = roman.romanNumeralFromChord(c, 'C')
>>> rn.addedSteps
[]

addedSteps should probably be promoted to Harmony so that it can be set when constructing ChordSymbols.

Then, I think a second issue is that I'm still puzzled why figure alterations on chromatic roots seem to be using the key of the chromatic root as the reference rather than the explicit key context given, which was the target of my diff above:

>>> c = chord.Chord('C3 E3 G3 A3')
>>> roman.romanNumeralFromChord(c, key.Key('a'))
<music21.roman.RomanNumeral i65 in a minor>

I would expect <music21.roman.RomanNumeral i6b5 in a minor> because of the G# in the key sig. Or are the figures not supposed to be interpreted like figured bass?

I think these can be solved independently.

jacobtylerwalls commented 3 years ago

Oh that's a third issue. In that last example I gave i6b5 was the result before #875. So some finessing to be had if we want it back. EDIT: ah, got it, this is part and parcel of the new desired behavior. But affecting i7 chords only, I guess?