Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

key_to_notes with an option to return only the notes from the scale #1828

Open
marcevrard opened this issue Apr 1, 2024 · 4 comments
Open
Labels
question Issues asking for help doing something

Comments

@marcevrard
Copy link

marcevrard commented Apr 1, 2024

I was a bit surprised at first when I used key_to_notes as I expected the default behavior to return only the seven notes of the scale (with possibly the option for pentatonic scales). I reckon it would be more consistent with the key_to_degrees function.

>>> librosa.key_to_notes("C:min")
['C', 'D', 'E♭', 'F', 'G', 'A♭', 'B♭']

Of course, it's possible to obtain the same result by resorting to an extra step with the key_to_degrees function:

>>> key = "C:min"
>>> degs = librosa.key_to_degrees(key)
>>> np.array(librosa.key_to_notes(key))[degs].tolist()
['C', 'D', 'E♭', 'F', 'G', 'A♭', 'B♭']
@lostanlen
Copy link
Contributor

Related: #1760 (now closed)

@bmcfee bmcfee added the question Issues asking for help doing something label Apr 1, 2024
@bmcfee
Copy link
Member

bmcfee commented Apr 1, 2024

The documentation does explicitly say in the first line:

def key_to_notes(key: str, *, unicode: bool = True, natural: bool= False) -> List[str]:
"""List all 12 note names in the chromatic scale, as spelled according to
a given key (major or minor) or mode (see below for details and accepted abbreviations).

Is there some way this could be made more clear?

The general idea in the API here is, as you've correctly identified, to separate the functionality of identifying the scale degrees belonging to a key (key_to_degrees) from the problem of spelling notes. This makes it possible, for example, to spell notes outside of the key, which would not be possible if the function behaved as you initially expected.

@lostanlen
Copy link
Contributor

lostanlen commented Apr 18, 2024

@marcevrard note that this works out of the box in music21

>>> from music21 import key
>>> [p.unicodeName for p in key.Key('Cm').pitches][:7]
['C', 'D', 'E♭', 'F', 'G', 'A♭', 'B♭']

https://web.mit.edu/music21/doc/usersGuide/usersGuide_15_key.html

@bmcfee
Copy link
Member

bmcfee commented Apr 19, 2024

I've been mulling this one over a bit, and considering the option of adding a shortcut to implement the degree slicing logic to return only the in-key notes. Having thought about it though, I suspect it's asking for trouble.

Think the potential confusion here stems from the fact that key_to_notes is not just returning a set of strings - it is a mapping of pitch class index to note names, i.e. order matters.

This distinction gets muddled a bit when using C as the tonic (eg in @marcevrard's original post). If we instead use a different tonic:

In [2]: librosa.key_to_notes("G:min", natural=True)
Out[2]: ['C', 'D♭', 'D', 'E♭', 'E♮', 'F', 'G♭', 'G', 'A♭', 'A', 'B♭', 'B♮']

Note that the result still starts on C (pitch class 0).

If we slice out the degrees of interest,

In [9]: degs = librosa.key_to_degrees("G:min")

In [10]: np.array(librosa.key_to_notes("G:min", natural=True))[degs]
Out[10]: array(['G', 'A', 'B♭', 'C', 'D', 'E♭', 'F'], dtype='<U2')

Note that it now starts on G because degs == [ 7, 9, 10, 0, 2, 3, 5] changes the order.

In plenty of applications, this is fine. However, we've now subtly changed the definition of what's being returned beyond just taking out a subset: the result is not valid as a mapping of pitch-class to name. (The fact that it was defined thusly before is what lets us get away with degree slicing in the first palce.)

This is a subtle point, but I think we should err on the side of keeping it simple and well-defined.

It may be worth expanding the documentation to more clearly state the intent of the function and add an example entry that shows how to slice degrees out for just the in-key notes. Does that seem reasonable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issues asking for help doing something
Development

No branches or pull requests

3 participants