3
\$\begingroup\$

Hello I'm doing a mini project that is modeled after pitch constellation. --> https://en.wikipedia.org/wiki/Chromatic_circle Basically, pitch constellation is modeled after a clock where each note is assigned a number. When the wheel goes clockwise it goes up the scale, and when it goes counterclockwise it goes down. I thought this might be a great idea for a midi project. (My project only goes up the scale.)

Here's how it works:

  1. It prompts for an input of a music note and then shifts the elements so that the list will start with the root key. Then it will return the notes starting with the root note or the note selected.

  2. Then it prompts to enter what mode to return the music note values. The way this done is by assigning the elements in the returned notes to the value selected in the dict variable scale. Which will then select the index number of the items and create a new list.

  3. Lastly, it returns the scale with the key and mode.

That's all it does so far. Eventually, I'd like to combine it with a midi module I found in pythons open library and then have it send midi out. I want to get honest feedback on what areas can be improved in my program before I go any further.



notes = ['C', 'C#', 'D', 'D#',
             'E', 'F', 'F#', 'G', 'G#',
             'A', 'A#','B']

scale = {'major':[0, 2, 4, 5, 7, 9, 11],
    'minor':[0, 2, 3, 5, 7, 8, 10],
    'phrygian':[0, 1, 3, 5, 7, 8, 10],
    'lydian':[0, 2, 4, 6, 7, 9, 11],
    'mixolydian':[0, 2, 4, 5, 7, 9, 10],
    'aeolian':[0, 2, 3, 5, 7, 8, 10],
    'locrian':[0, 1, 3, 5, 6, 8, 10],
    'chromatic':[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11],
    'wholetone':[0, 2, 4, 6, 8, 10],
    'diminished7thchord': [0, 3, 6, 9],
    'augmentedchord':[0, 4, 8],
    'tritone':[0, 6]}


rootkey = input("Enter the root Note: ").upper()

class CircleFifths:
    """docstring for CircleFifths."""
    def __init__(self, key):
        self.rootkey = rootkey

    def shift(self, rootkey, index=0, s=0,):
        #shifts the leading item (rootkey) entered by the user down the list
        #removes the last term and puts it as the first term
        while index < 12 and s <12:
            s += 1
            index += 1
            new_notes = notes[-s:] + notes[:-s]
            if rootkey == new_notes[0]:
                print(new_notes)
                return new_notes



def main():
    a = CircleFifths(rootkey)
    key = a.shift(rootkey)

    keymode = []

    print('Select a scale and enter it below')
    for k in scale:
        print(k)

    mode = input("Enter the mode: ").lower()
    try:
        x = scale.get(mode)
        keymode = [key[i] for i in x]
        print(keymode)
    except:
        if mode not in scale:
            print("Scale not in list. Please start the program again.")
        return

main()
\$\endgroup\$
2
  • \$\begingroup\$ why did you import random? \$\endgroup\$ – ppwater Dec 15 '20 at 7:20
  • \$\begingroup\$ My apologies. I forgot to take that out. \$\endgroup\$ – manny_22 Dec 15 '20 at 17:02
2
\$\begingroup\$

Immutable sequences

Convert notes and the values in scale from list literals [] to tuple literals () since it's expected that they cannot and will not change.

Musical theory

Consider distinguishing between harmonic and melodic minor, which are different scales.

Global code

Move

rootkey = input("Enter the root Note: ").upper()

into your main(). Also, your constructor

def __init__(self, key):
    self.rootkey = rootkey

needs to refer to its key argument and not the rootkey global.

Shadowing

def shift(self, rootkey, index=0, s=0,):

has a number of issues:

  • You're shadowing the global rootkey with an identically-named argument
  • You shouldn't have a rootkey argument at all, since it's already a member on self

Loop like a native

Consider renaming index and s to first_index and first_s, so that you can replace your while with

for index, s in zip(
    range(first_index, 12),
    range(first_s, 12)
):

Inner prints

Delete the print from here:

            print(new_notes)
            return new_notes

If you want to print it, do so at the outer level.

Logic by exception

Consider writing

mode = input("Enter the mode: ").lower()
try:
    x = scale.get(mode)
    keymode = [key[i] for i in x]
    print(keymode)
except:
    if mode not in scale:
        print("Scale not in list. Please start the program again.")
    return

as

mode_name = input("Enter the mode: ").lower()
mode = scale.get(mode_name)
if mode is None:
    print(f"Scale {mode_name} not in list. Please start the program again.")
else:
    key_mode = [key[i] for i in mode]
    print(key_mode)

In other words, where possible do not rely on exceptions (especially with no type specified) to govern your logic; and avoid a double-dictionary lookup by using get.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.