Improve test coverage in key.py

pyabc2/key.py

@@ -186,7 +186,12 @@ def _scale_intervals(
         Whether to return 7 intervals by computing the interval from scale degree 7 to 8,
         by default True.
     """
-    assert len(values) == 7
+    if len(values) != 7:
+        raise ValueError(f"expected 7 values, got {len(values)}")
+    if values[0] != 0:
+        raise ValueError(f"first value should be 0, not {values[0]}")
+    if not values[-1] < 12:
+        raise ValueError(f"last value should be < 12, not {values[-1]}")
 
     if include_upper:
         values.append(12)
@@ -199,7 +204,7 @@ def _scale_intervals(
         elif dv == 1:
             interval = "H"
         else:
-            raise ValueError("strange interval (not W/H)")
+            raise ValueError(f"strange interval {dv} (not W/H)")
         intervals.append(interval)
     # TODO: should make a class for interval!
 
@@ -260,15 +265,18 @@ def __init__(
         mode
             Mode specification, (e.g., ``maj``, ``min``, ``dor``, ...).
         """
+        msg = "pass either just `name` or both `tonic` and `mode`"
         if name is not None:
-            assert tonic is None and mode is None, "pass either `name` or `tonic`+`mode`"
+            if not (tonic is None and mode is None):
+                raise ValueError(msg)
             # Handle occasional `K:` line used to indicate default key (C) and tune start
             if name == "":
                 name = "C"
             self.tonic, self._mode = Key.parse_key(name)
         else:
             # TODO: default mode to major for consistency?
-            assert tonic is not None and mode is not None, "pass either `name` or `tonic`+`mode`"
+            if not (tonic is not None and mode is not None):
+                raise ValueError(msg)
             self.tonic = PitchClass.from_name(tonic)
             self._mode = _validate_and_normalize_mode_name(mode)
 
@@ -304,8 +312,8 @@ def parse_key(key: str) -> Tuple[PitchClass, str]:
 
         try:
             mode = _validate_and_normalize_mode_name(mode)
-        except ValueError:
-            raise ValueError("Unrecognized mode specification '{mode}' from key '{key}'")
+        except ValueError as e:
+            raise ValueError(f"Unrecognized mode specification '{mode}' from key '{key}'") from e
 
         return PitchClass.from_name(base + acc), mode
         # TODO: probably should either return a Key or be private / maybe outside class
@@ -350,18 +358,21 @@ def relative(self, mode: str, *, match_acc: bool = False) -> "Key":
         tonic_ef = tonic.equivalent_flat
 
         if match_acc:
-            # Select flat or sharp to match the current key name
-
-            # TODO: PitchClass from value with acc option?
-            if "#" in key.name:
-                if tonic_es.acc == "#":  # single sharp used
-                    tonic_name = tonic_es.name
-            elif "b" in key.name:
-                if tonic_ef.acc == "b":  # single flat used
-                    tonic_name = tonic_ef.name
+            # Select accidentals to match the current key name
+            # (haven't identified a case where a successful result is different than
+            # the default approach below though)
+
+            # Note = is allowed for explicit natural, but it wouldn't be added automatically
+            orig_acc = key.acc.replace("=", "")
+            for tonic_cand in [tonic_es, tonic_ef, tonic]:
+                if tonic_cand.acc == orig_acc:
+                    tonic_name = tonic_cand.name
+                    break
             else:
-                tonic_name = tonic.name
-
+                raise ValueError(
+                    f"unable to match accidentals ({key}{mode0} -> "
+                    f"{{{tonic_es}, {tonic_ef}, {tonic}}}{mode})"
+                )
         else:
             # Use the letter that it should be in the scale
             sd0 = MODE_SCALE_DEGREE[mode0]
@@ -376,8 +387,8 @@ def relative(self, mode: str, *, match_acc: bool = False) -> "Key":
                     tonic_name = tonic_es.name
                 elif tonic_ef.nat == new_nat:
                     tonic_name = tonic_ef.name
-                else:
-                    raise Exception
+                else:  # pragma: no cover
+                    raise AssertionError
             else:
                 tonic_name = tonic.name
 

pyabc2/pitch.py

@@ -236,6 +236,8 @@ def from_name(cls, name: str) -> "PitchClass":
 
         return pc
 
+    # TODO: PitchClass from value with acc option (to hint name)?
+
     @property
     def equivalent_sharp(self) -> "PitchClass":
         pcnew = self - 1

tests/test_key.py

@@ -6,7 +6,13 @@
 
 import pytest
 
-from pyabc2.key import CMAJ_LETTERS, MODE_VALUES, Key, _mode_chromatic_scale_degrees
+from pyabc2.key import (
+    CMAJ_LETTERS,
+    MODE_VALUES,
+    Key,
+    _mode_chromatic_scale_degrees,
+    _scale_intervals,
+)
 
 
 def many_possible_key_name_inputs():
@@ -39,20 +45,70 @@ def test_parse_key_basic_succeeds(key_name):
     Key(name=key_name)
 
 
-def test_key_sig():
+def test_default_key():
+    k = Key("")
+    assert k.tonic.name == "C"
+
+
+def test_arg_mix_fails():
+    with pytest.raises(ValueError, match="pass either just `name` or both `tonic` and `mode`"):
+        Key(name="C", tonic="D")
+
+
+def test_tonic_mode_both_needed():
+    # Maybe will default to major in the future, but currently requiring both
+    # be passed and mode be valid.
+    with pytest.raises(ValueError, match="pass either just `name` or both `tonic` and `mode`"):
+        Key(tonic="C")
+
+
+def test_parse_key_invalid_base_fails():
+    with pytest.raises(ValueError, match="Invalid key specification"):
+        Key.parse_key("X")
+
+
+def test_parse_key_invalid_mode_fails():
+    with pytest.raises(ValueError, match="Unrecognized mode specification"):
+        with pytest.warns(UserWarning, match="extra info"):
+            Key.parse_key("Bad mode + extras warning")
+
+
+def test_sharp_key_sig():
     k = Key("Amaj")
     assert k.key_signature == ["F#", "C#", "G#"]
     for n in "FCG":
         assert k.accidentals[n] == "#"
 
 
+def test_flat_key_sig():
+    k = Key("Eb")
+    assert k.key_signature == ["Bb", "Eb", "Ab"]
+    for n in "BEA":
+        assert k.accidentals[n] == "b"
+
+
 def test_relatives():
     Am = Key("Am")
     C = Key("C")
     assert Am.relative("ionian") == Am.relative_major == C
     assert C.relative("aeolian") == C.relative_minor == Am
     assert Key("Ador").relative_major == Key("G")
 
+    assert Am.relative("major", match_acc=True) == C
+    assert Key("Cb").relative("dorian") == Key("Dbdor")
+    assert Key("Cb").relative("dorian", match_acc=True) == Key("Dbdor")
+    assert Key("C#").relative("dorian", match_acc=True) == Key("D#dor")
+    assert Key("C#").relative("dorian") == Key("D#dor")
+
+    # Note key spec parsing currently only supports single #/b,
+    # while pitch class parsing (used when passing tonic) supports up to 2.
+    assert Key("Cbdorian").relative_major == Key(tonic="Bbb", mode="Major")
+    with pytest.raises(ValueError, match="unable to match accidentals"):
+        Key("Cbdorian").relative("Major", match_acc=True)
+    assert Key("E#major").relative("Dorian") == Key(tonic="F##", mode="Dorian")
+    with pytest.raises(ValueError, match="unable to match accidentals"):
+        Key("E#major").relative("Dorian", match_acc=True)
+
 
 @pytest.mark.parametrize(
     ("mode", "acc_format"), [(m, a) for m, a in product(MODE_VALUES, ["#", "b", "#/b", "b/#"])]
@@ -63,6 +119,16 @@ def test_mode_chromatic_scale_degrees(mode, acc_format):
     assert all(len(s) in [1, 2, 5] and s[-1:-2:-1] in "1234567" for s in csds)
 
 
+def test_invalid_mode_chromatic_scale_degrees_bad_mode():
+    with pytest.raises(ValueError, match="invalid mode name"):
+        _mode_chromatic_scale_degrees("invalid")
+
+
+def test_invalid_mode_chromatic_scale_degrees_bad_acc_fmt():
+    with pytest.raises(ValueError, match="invalid `acc_format`"):
+        _mode_chromatic_scale_degrees("ionian", acc_fmt="invalid")
+
+
 @pytest.mark.parametrize("mode", MODE_VALUES)
 def test_mode_scale_degrees_wrt_major(mode):
     # They should be the same no matter the tonic is.
@@ -101,3 +167,71 @@ def test_scvs_consistency():
     scvs0 = key.scale_chromatic_values
     scvs = [pc.value_in(key) for pc in key.scale]
     assert scvs0 == scvs
+
+
+def test_str():
+    assert str(Key("C")) == "Cmaj"
+
+
+def test_repr():
+    assert repr(Key("C")) == "Key(tonic=C, mode='Major')"
+
+
+def test_inequality():
+    assert Key("C") != "this is not a Key"
+
+
+def test_whole_tone_scale_intervals_fails():
+    with pytest.raises(ValueError, match=r"expected 7 values, got 6"):
+        _scale_intervals([0, 2, 4, 6, 8, 10])
+
+
+def test_nonzero_start_fails():
+    with pytest.raises(ValueError, match=r"first value should be 0"):
+        _scale_intervals([1] * 7)
+
+
+def test_beyond_octave_fails():
+    with pytest.raises(ValueError, match=r"last value should be < 12"):
+        _scale_intervals([0, 2, 4, 5, 7, 9, 13])
+
+
+def test_scale_with_large_interval_fails():
+    with pytest.raises(ValueError, match=r"strange interval 3 \(not W/H\)"):
+        _scale_intervals([0, 2, 3, 5, 6, 9, 10])
+
+
+def test_print_scale(capsys):
+    Key("C").print_scale()
+    output = capsys.readouterr().out
+    assert output == "C  D  E  F  G  A  B \n"
+
+
+def test_print_scale_degrees_wrt_major(capsys):
+    Key("C").print_scale_degrees_wrt_major()
+    output = capsys.readouterr().out
+    assert output == "1  2  3  4  5  6  7 \n"
+
+
+def test_print_chromatic_scale_degrees(capsys):
+    Key("C").print_chromatic_scale_degrees()
+    output = capsys.readouterr().out
+    assert output == "1  #1 2  #2 3  4  #4 5  #5 6  #6 7 \n"
+
+
+def test_print_scale_chromatic_values(capsys):
+    Key("C").print_scale_chromatic_values()
+    output = capsys.readouterr().out
+    assert output == "0  2  4  5  7  9  11\n"
+
+
+def test_print_intervals_wh(capsys):
+    Key("C").print_intervals()
+    output = capsys.readouterr().out
+    assert output == "W W H W W W H\n"
+
+
+def test_print_intervals_dash(capsys):
+    Key("C").print_intervals(fmt="-")
+    output = capsys.readouterr().out
+    assert output == "|--|--|-|--|--|--|-\n"