Skip to content

Mode.Parse("ionian") != Mode.Ionian, so a parsed "C ionian" key is analyzed with the minor-key tables #290

Description

@matt-edmondson

What's wrong

Mode.Ionian is an alias that returns Major, whose Name is "major", and ModeTests asserts Mode.Major == Mode.Ionian. But Mode.TryParse (Semantics.Music/Mode.cs:178) builds new() { Name = name.ToLowerInvariant() }, so parsing "ionian" gives a mode whose Name is "ionian". Record equality compares Name, so the parsed value is not equal to Mode.Ionian or Mode.Major.

Code that branches on key.Mode == Mode.Major then treats a parsed Ionian key as minor. For example, Semantics.Music/Progression.Chromatic.cs lines 55 and 68 pick the secondary-dominant table and the parallel mode this way.

Repro

Mode.Parse("ionian") == Mode.Ionian;                                   // False
Key ionian = Key.Parse("C ionian");
ionian == Key.Create(PitchClass.Create(0), Mode.Major);                // False
Progression.Parse("4/4 C D7 G C").ChromaticChords(ionian)[0].Detail;   // "V/v"   (with Mode.Major: "V/V")
Progression.Parse("4/4 C Fm C").ChromaticChords(ionian)[0];            // Chromatic, Detail = null
                                                                        // (with Mode.Major: borrowed "from parallel minor")

This was reproduced with a temporary MSTest on net10.0 at 8c06aba.

Why it matters

"C ionian" and "C major" are the same key, and the static API already says they are equal. Analysis that goes through Parse gives different results depending on which spelling the user typed. Keys parsed from the same text also don't compare equal to keys built in code.

Suggested fix / acceptance criteria

  • Canonicalize aliases in TryParse, for example with a small alias table mapping "ionian" to "major". Check the other alias properties on Mode too, such as Aeolian versus Minor, if any exist.
  • Assert.AreEqual(Mode.Ionian, Mode.Parse("ionian")) and Assert.AreEqual(Key.Parse("C major"), Key.Parse("C ionian")) pass.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions