Improve chord detection (and remove invalid chord) (tonaljs#209)

docs/CONTRIBUTING.md

@@ -24,10 +24,13 @@ yarn build
 
 Modules are written in Typescript and live inside `packages/` folder.
 
-Some guidelines:
+If you are adding new functionality or fixing a bug, please add a test for it.
 
-- If you are adding new functionality to a current module, please add a test for it.
-- Ensure all tests passes and library can be built, before making a pull request: `yarn test:ci`
+**Run test and build the library before submitting a pull request :pray:**
+
+```bash
+yarn test:ci
+```
 
 #### How to add a new module
 
@@ -37,6 +40,6 @@ To create a new module:
 - Add a new package.json inside the folder (see any of them as an example)
 - Add required dependencies to "dependencies" inside package.json. Ensure correct dependency versions. For example, if your module needs to use `tonal/core` look at core's package.json to see what version to use
 - After adding your dependencies, use lerna to wire them up: run `yarn lerna` at root folder
-- Add your functionallity and tests
+- Add your functionality and tests
 - Ensure everything works: run `yarn test:ci` at root folder
 - Create a pull request

package.json

@@ -14,9 +14,10 @@
     "build:packages": "lerna exec --parallel  -- rollup -c=../../rollup.config.js",
     "build:browser": "cd packages/tonal && rollup -c=rollup.browser.config.js",
     "build:clean": "lerna exec --parallel -- rimraf dist/",
+    "format": "prettier --write packages/**/*.ts",
     "test": "jest --coverage",
     "lint": "tslint --fix --project tsconfig.json -t codeFrame 'packages/**/*.ts'",
-    "test:ci": "yarn lint && yarn build && yarn test -- --no-cache"
+    "test:ci": "yarn format && yarn lint && yarn build && yarn test -- --no-cache"
   },
   "husky": {
     "hooks": {

packages/chord-detect/index.ts

@@ -1,4 +1,4 @@
-import { get as chordType } from "@tonaljs/chord-type";
+import { all } from "@tonaljs/chord-type";
 import { note } from "@tonaljs/core";
 import { modes } from "@tonaljs/pcset";
 
@@ -7,8 +7,6 @@ interface FoundChord {
   readonly name: string;
 }
 
-const NotFound: FoundChord = { weight: 0, name: "" };
-
 const namedSet = (notes: string[]) => {
   const pcToName = notes.reduce<Record<number, string>>((record, n) => {
     const chroma = note(n).chroma;
@@ -39,21 +37,29 @@ function findExactMatches(notes: string[], weight: number): FoundChord[] {
   const tonic = notes[0];
   const tonicChroma = note(tonic).chroma;
   const noteName = namedSet(notes);
+  // we need to test all chormas to get the correct baseNote
   const allModes = modes(notes, false);
 
-  const found: FoundChord[] = allModes.map((mode, chroma) => {
-    const chordName = chordType(mode).aliases[0];
-    if (!chordName) {
-      return NotFound;
-    }
-    const baseNote = noteName(chroma);
-    const isInversion = chroma !== tonicChroma;
-    if (isInversion) {
-      return { weight: 0.5 * weight, name: `${baseNote}${chordName}/${tonic}` };
-    } else {
-      return { weight: 1 * weight, name: `${baseNote}${chordName}` };
-    }
+  const found: FoundChord[] = [];
+  allModes.forEach((mode, index) => {
+    // some chords could have the same chroma but different interval spelling
+    const chordTypes = all().filter((chordType) => chordType.chroma === mode);
+
+    chordTypes.forEach((chordType) => {
+      const chordName = chordType.aliases[0];
+      const baseNote = noteName(index);
+      const isInversion = index !== tonicChroma;
+      if (isInversion) {
+        found.push({
+          weight: 0.5 * weight,
+          name: `${baseNote}${chordName}/${tonic}`,
+        });
+      } else {
+        found.push({ weight: 1 * weight, name: `${baseNote}${chordName}` });
+      }
+    });
   });
+
   return found;
 }
 

packages/chord-detect/test.ts

@@ -1,4 +1,3 @@
-// tslint:disable-next-line: no-implicit-dependencies
 import { detect } from "./index";
 
 describe("@tonal/chord-detect", () => {
@@ -9,6 +8,10 @@ describe("@tonal/chord-detect", () => {
     expect(detect(["E", "G#", "B", "C#"])).toEqual(["E6", "C#m7/E"]);
   });
 
+  test("(regression) detect aug", () => {
+    expect(detect(["C", "E", "G#"])).toEqual(["Caug", "Eaug/C", "G#aug/C"]);
+  });
+
   test("edge cases", () => {
     expect(detect([])).toEqual([]);
   });

packages/chord-type/data.ts

@@ -68,7 +68,6 @@ const CHORDS: string[][] = [
   ],
   // ==Legacy==
   ["1P 2M 4P 5P", "", "sus24 sus4add9"],
-  ["1P 3M 13m", "", "Mb6"],
   ["1P 3M 5A 7M 9M", "", "maj9#5 Maj9#5"],
   ["1P 3M 5A 7m", "", "7#5 +7 7+ 7aug aug7"],
   ["1P 3M 5A 7m 9A", "", "7#5#9 7#9#5 7alt"],

packages/chord-type/test.ts

@@ -1,6 +1,8 @@
 import ChordType from "./index";
+import { interval } from "@tonaljs/core";
+import DATA from "./data";
 
-const $ = (str: string) => str.split(" ");
+const INTERVALS = DATA.map((d) => d[0]).sort();
 
 describe("@tonaljs/chord-type", () => {
   test("names", () => {
@@ -24,7 +26,7 @@ describe("@tonaljs/chord-type", () => {
   });
 
   test("all returns all chords", () => {
-    expect(ChordType.all()).toHaveLength(107);
+    expect(ChordType.all()).toHaveLength(106);
   });
 
   test("get ", () => {
@@ -55,3 +57,38 @@ describe("@tonaljs/chord-type", () => {
     expect(ChordType.keys()).toEqual([]);
   });
 });
+
+describe("@tonal/chord-type data", () => {
+  test("no repeated intervals", () => {
+    for (let i = 1; i < INTERVALS.length; i++) {
+      expect(INTERVALS[i - 1]).not.toEqual(INTERVALS[i]);
+    }
+  });
+
+  test("all chords must have abreviatures", () => {
+    DATA.forEach((data) => {
+      const abrrvs = data[2].trim();
+      expect(abrrvs.length).toBeGreaterThan(0);
+    });
+  });
+
+  test("intervals should be in ascending order", () => {
+    DATA.forEach((data) => {
+      const [list] = data;
+      const intervals = list.split(" ").map((i) => interval(i).semitones || 0);
+
+      try {
+        for (let i = 1; i < intervals.length; i++) {
+          expect(intervals[i - 1]).toBeLessThan(intervals[i]);
+        }
+      } catch (e) {
+        // tslint:disable-next-line
+        console.error(
+          `Invalid chord: intervals should be in ascending order`,
+          data
+        );
+        throw e;
+      }
+    });
+  });
+});