titus-ong / chordparser

A Python 3 package that provides a musical framework to analyse chords
MIT License
12 stars 2 forks source link

Scale in ChordRomanConverter #40

Closed amelie106 closed 1 year ago

amelie106 commented 1 year ago

What a great library, thank you! I used it in a project for university and it helped me a lot :) Something I noticed: in ChordRomanConverter, the scale is hardcoded to 'major', which resulted in some errors in my analysis. I changed it as follows:

from this:

if isinstance(scale_key, Scale):
      scale_root = scale_key.key.root
else:
    scale_root = scale_key.root
scale = self._SE.create_scale(scale_root, "major")

to this:

if isinstance(scale_key, Scale):
    scale_root = scale_key.key.root
    scale = self._SE.create_scale(scale_root, scale_key.key.mode)
else:
    scale_root = scale_key.root
    scale = self._SE.create_scale(scale_root, scale_key.mode)

Happy to hear your thoughts!

titus-ong commented 1 year ago

Hey, glad it helped!

You are referring to this section, correct? https://github.com/titus-ong/chordparser/blob/2c1d0d85528c4b51c749cdc17662f55415679284/src/chordparser/editors/chord_roman_converter.py#L86-L90

I'll admit my theory knowledge isn't the best, I've only seen Roman numerals for chords in the major mode hence I hard-coded it to major. Could you give an example of some chord conversions you are doing that the current code gets wrong?

amelie106 commented 1 year ago

Hey, yes that is exactly the part I was talking about :)

To give an example, if there is a progression of Am -> C -> Dm -> E -> Em in a key of A Minor, with a scale hardcoded to major it results in the following: i -> bIII -> iv -> V -> v whereas it should be: i -> III -> iv -> V -> v

The bIII appears because C is not part of the A Major scale, C# is.

Another example: B Minor scale, the progression is Bm -> G -> D -> A in a Major context (hardcoded): i -> bVI -> bIII -> bVII in a Minor context: i -> VI -> III -> VII, which is correct according to music theory

If you wanna read up on it, here is a link: https://www.musicnotes.com/now/tips/roman-numeral-analysis-in-music/ Also don't hesitate to ask if you have any other questions :)

Hope I could help! All the best, Amelie

titus-ong commented 1 year ago

Thanks for the explanation! Hm my understanding is correct, but I'm guessing I confused the chord's quality with the scale's quality while writing the code.

Anyway, thanks for flagging this out, Amelie! I'm quite impressed you knew which lines of code to change, I still don't fully understand my code when I was checking out your changes haha.

Do you want to create a pull request for this so your contribution will be recognised? I haven't touched this repository in quite a while because of work and I don't have any plans to update it for now, but I'll help to merge your PR in and publish a new version.

amelie106 commented 1 year ago

Sure, I will make a PR! Also, if you ever plan to make a similar project, I would love to contribute - let me know :)

titus-ong commented 1 year ago

Cool, I would honestly love to get back to this project some day, will definitely let you know then.

titus-ong commented 1 year ago

Apologies for the late merge. Thankfully my CD workflow still works so the package was published successfully, you can check it out here or just install the latest version (should be 0.4.2). Thanks again!

amelie106 commented 1 year ago

Great! Happy to contribute :)