A NaN constructor option is the option left out; CabinetSim keeps an echoed macro

lib/audioeffects/_component.py

@@ -613,6 +613,34 @@ def let_go_of_source(head, source, consumers=()):
 # Metadata
 
 
+def _is_nan(value):
+    return isinstance(value, float) and value != value
+
+
+def _without_nan(cls, options, keywords):
+    """The constructor's options with every NaN taken out, so `_build` sees
+    that option as left out and uses its default, as each class's
+    documentation says. One rule here covers every class; none has to clean
+    its own arguments.
+
+    A class that gives NaN a meaning of its own for an option (NaN Glide is
+    the jump, NaN damping is out of circuit, NaN interval is refused) names
+    it in `_NAN_OWN`, and that option reaches `_build` as passed.
+
+    A NaN passed by position is dropped by name where the interpreter can
+    name `_build`'s arguments (CPython); on one that cannot, only the
+    keyword form is cleaned."""
+    own = cls._NAN_OWN
+    if any(_is_nan(value) for value in options):
+        code = getattr(cls._build, "__code__", None)
+        if code is not None and len(options) <= code.co_argcount - 1:
+            names = code.co_varnames[1:1 + len(options)]
+            keywords = dict(zip(names, options), **keywords)
+            options = ()
+    return options, {name: value for name, value in keywords.items()
+                     if name in own or not _is_nan(value)}
+
+
 class MetadataError(ValueError):
     """A class's macro or patch tables do not have the frozen shape."""
 
@@ -800,6 +828,10 @@ class Component:
     _MACRO_RANGES = ()
     PATCHES = {}
 
+    #: Constructor options for which NaN means something the class defines
+    #: (see `_without_nan`); every other NaN option is left out.
+    _NAN_OWN = ()
+
     #: Every `self._output = node` in every subclass re-points one stable
     #: port. See `output_port` at the top of this module.
     _output = output_port
@@ -830,6 +862,8 @@ def __init__(self, source, *options, **keywords):
             raise ValueError("an effect source must be 1 or 2 channels, not "
                              "%d" % source_channels)
 
+        options, keywords = _without_nan(cls, options, keywords)
+
         self._source = source
         self._sample_rate = sample_rate
         self._channel_count = source_channels

lib/audioeffects/cabinetsim.py

@@ -619,13 +619,27 @@ class a finer position is not safe to honour, because only those
         done here, on the MIDI number the host passed, where `+ 0.5` is
         exact in both precisions - snapping later, from the stored 0..1
         position, would put the tie back at every half-step.
-        `get_macro` reads back the snapped number, not the one sent.
+        `get_macro` reads back the snapped number, not the one sent. The one
+        exception is the number `get_macro` itself returns: sent straight
+        back it is kept, so a constructor option off the grid survives a
+        host's echo.
         """
         try:
             value = _component.clamped_midi(value)
         except (TypeError, ValueError):
             pass
         else:
+            if (isinstance(index, int) and not isinstance(index, bool)
+                    and 0 <= index < len(self._macros)
+                    and not self._deinited
+                    and value == self.get_macro(index)):
+                # A host echoing `get_macro()` back is not choosing a new
+                # position: a constructor option off the grid keeps the
+                # exact position it was built at, and the same number
+                # plays the same filter (the echo of 46.99 was 47).
+                return _component.Component.set_macro(
+                    self, index, value, channel=channel, note_id=note_id,
+                    sample_position=sample_position)
             # The clamp first, so +inf is the top stop and never reaches
             # int(); NaN fails `>= 0` and is the bottom stop, as in the base.
             value = (float(int(min(value, 127.0) + 0.5)) if value >= 0.0

lib/audioeffects/convolutionreverb.py

@@ -275,6 +275,8 @@ class ConvolutionReverb(_component.Component):
     """
 
     NAME = 'ConvolutionReverb'
+    #: NaN is this option's own stop, not the default (documented above).
+    _NAN_OWN = ('damping_hz',)
     DISPLAY_NAME = 'Convolution Reverb'
     CATEGORIES = ('Reverb',)
     VERSION = '0.1.0'

lib/audioeffects/digitaldelay.py

@@ -327,6 +327,8 @@ class DigitalDelay(_component.Component):
     audiodsp tier; zero latency. The module docstring has the rest."""
 
     NAME = 'DigitalDelay'
+    #: NaN is this option's own stop, not the default (documented above).
+    _NAN_OWN = ('glide_ms',)
     DISPLAY_NAME = 'Digital Delay'
     CATEGORIES = ('Delay',)
     VERSION = '0.1.0'

lib/audioeffects/shimmerhall.py

@@ -231,6 +231,8 @@ class ShimmerHall(_component.Component):
     page."""
 
     NAME = 'ShimmerHall'
+    #: NaN is this option's own stop, not the default (documented above).
+    _NAN_OWN = ('interval',)
     DISPLAY_NAME = 'Shimmer Hall'
     CATEGORIES = ('Effect Rack', 'Reverb')
     VERSION = '0.1.0'

lib/audioeffects/tapedelay.py

@@ -336,6 +336,8 @@ class TapeDelay(_component.Component):
     The module docstring is the player's manual."""
 
     NAME = 'TapeDelay'
+    #: NaN is this option's own stop, not the default (documented above).
+    _NAN_OWN = ('glide_ms',)
     DISPLAY_NAME = 'Tape Delay'
     CATEGORIES = ('Delay',)
     VERSION = '0.1.0'

tests/support/kit_planted.py

@@ -527,9 +527,12 @@ class CtorNan(KitPlain):
     the bottom stop."""
     NAME = "CtorNan"
 
-    def _build(self, mix=1.0, gain=1.0, patch=None):
+    def __init__(self, source, *options, **keywords):
+        # The base class drops a NaN option before `_build` runs, so the
+        # fault has to be planted where the NaN still shows.
+        gain = keywords.get("gain", 1.0)
         self._nan = gain != gain
-        KitPlain._build(self, mix, gain, patch)
+        KitPlain.__init__(self, source, *options, **keywords)
 
     def _level(self):
         if self._nan:

tests/test_nan_options.py

@@ -0,0 +1,137 @@
+"""A NaN constructor option is the option left out, on every effect.
+
+The API reference promises it for macros (NaN is the bottom stop) and each
+class's docstring promises it for constructor options (NaN takes the
+option's default). 28 classes built with the NaN as it stood. The base class
+now drops a NaN option before `_build` sees it, so one rule covers every
+class, and an echo of `get_macro()` through `set_macro()` leaves a macro
+where the constructor put it.
+"""
+
+import array
+import inspect
+import math
+import os
+import sys
+import unittest
+
+_HERE = os.path.dirname(os.path.abspath(__file__))
+sys.path.insert(0, os.path.join(_HERE, "support"))
+sys.path.insert(0, os.path.join(_HERE, "..", "lib"))
+
+import audiocore                                            # noqa: E402
+import audioeffects                                         # noqa: E402
+import kit_common                                           # noqa: E402
+
+NAN = float("nan")
+FRAMES = 2048
+
+
+def source():
+    values = [int(8000 * math.sin(i * 0.05)) for i in range(4096)] * 40
+    return audiocore.RawSample(array.array("h", values), sample_rate=48000,
+                               channel_count=2)
+
+
+def render(effect):
+    try:
+        return kit_common.render(effect, FRAMES)
+    finally:
+        effect.deinit()
+
+
+def options_of(cls):
+    """The names of `cls`'s numeric constructor options: those whose
+    default is a number or None (left out)."""
+    names = []
+    for p in inspect.signature(cls._build).parameters.values():
+        if p.name in ("self", "patch") or p.kind in (p.VAR_KEYWORD,
+                                                      p.VAR_POSITIONAL):
+            continue
+        if isinstance(p.default, bool):
+            continue
+        if p.default is None or isinstance(p.default, (int, float)):
+            names.append(p.name)
+    return [n for n in names if n not in cls._NAN_OWN]
+
+
+class ANanOptionIsTheOptionLeftOut(unittest.TestCase):
+
+    def test_every_class_renders_a_nan_option_as_its_default(self):
+        for name in audioeffects.ALL:
+            cls = getattr(audioeffects, name)
+            for option in options_of(cls):
+                with self.subTest(effect=name, option=option):
+                    try:
+                        left_out = render(cls(source()))
+                    except Exception:
+                        self.skipTest("the default build is not renderable")
+                    nan = render(cls(source(), **{option: NAN}))
+                    self.assertEqual(nan, left_out)
+
+    def test_a_positional_nan_is_the_option_left_out(self):
+        for name in ("Chorus", "Limiter", "Phaser"):
+            cls = getattr(audioeffects, name)
+            with self.subTest(effect=name):
+                self.assertEqual(render(cls(source(), NAN)),
+                                 render(cls(source())))
+
+    def test_a_class_keeps_the_nan_meaning_it_documents(self):
+        self.assertEqual(
+            {name: getattr(audioeffects, name)._NAN_OWN
+             for name in audioeffects.ALL
+             if getattr(audioeffects, name)._NAN_OWN},
+            {"ShimmerHall": ("interval",), "ConvolutionReverb":
+             ("damping_hz",), "TapeDelay": ("glide_ms",),
+             "DigitalDelay": ("glide_ms",)})
+
+    def test_a_number_still_plays_as_passed(self):
+        cls = audioeffects.Chorus
+        self.assertNotEqual(render(cls(source(), rate=2.5)),
+                            render(cls(source())))
+
+
+class AnEchoOfGetMacroLeavesTheMacroWhereItIs(unittest.TestCase):
+
+    OFF_GRID = {
+        "CabinetSim": dict(low_cut=80.0, body=0.37, break_up=2431.0,
+                           bite=4.7, top=5110.0, air=-21.3, mix=0.333),
+        "SlapbackDelay": dict(time_ms=97.3, level=0.37, saturation=0.31,
+                              tone_hz=3333.0, wow_cents=11.1, repeats=0.27),
+    }
+
+    def test_the_value_reads_back_exactly(self):
+        for name, options in self.OFF_GRID.items():
+            cls = getattr(audioeffects, name)
+            effect = cls(source(), **options)
+            try:
+                for index in range(len(cls.MACRO_LABELS)):
+                    with self.subTest(effect=name, macro=index):
+                        before = effect.get_macro(index)
+                        effect.set_macro(index, before, channel=-1)
+                        self.assertEqual(effect.get_macro(index), before)
+            finally:
+                effect.deinit()
+
+    def test_the_echo_does_not_change_the_sound(self):
+        for name, options in self.OFF_GRID.items():
+            cls = getattr(audioeffects, name)
+            for index in range(len(cls.MACRO_LABELS)):
+                with self.subTest(effect=name, macro=index):
+                    echoed = cls(source(), **options)
+                    echoed.set_macro(index, echoed.get_macro(index),
+                                     channel=-1)
+                    self.assertEqual(render(echoed),
+                                     render(cls(source(), **options)))
+
+    def test_cabinet_sim_still_snaps_a_value_the_host_chose(self):
+        effect = audioeffects.CabinetSim(source())
+        try:
+            effect.set_macro(2, 46.99)
+            self.assertEqual(effect.get_macro(2), 47.0)
+        finally:
+            effect.deinit()
+
+
+if __name__ == "__main__":
+    unittest.main()