From aabaf13d9efe67956231c12cf81b1c2a1f146f61 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:59:10 -0500 Subject: [PATCH 1/2] Components: a NaN constructor option is the option left out; CabinetSim keeps a macro echoed back from get_macro (#145) --- lib/audioeffects/_component.py | 25 +++++++ lib/audioeffects/cabinetsim.py | 16 ++++- tests/test_nan_options.py | 128 +++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 tests/test_nan_options.py diff --git a/lib/audioeffects/_component.py b/lib/audioeffects/_component.py index 3bf05eb7..5221dd12 100644 --- a/lib/audioeffects/_component.py +++ b/lib/audioeffects/_component.py @@ -613,6 +613,29 @@ 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 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.""" + 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 not _is_nan(value)} + + class MetadataError(ValueError): """A class's macro or patch tables do not have the frozen shape.""" @@ -830,6 +853,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 diff --git a/lib/audioeffects/cabinetsim.py b/lib/audioeffects/cabinetsim.py index 52f2cc35..88cfe2be 100644 --- a/lib/audioeffects/cabinetsim.py +++ b/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 diff --git a/tests/test_nan_options.py b/tests/test_nan_options.py new file mode 100644 index 00000000..94f45bdc --- /dev/null +++ b/tests/test_nan_options.py @@ -0,0 +1,128 @@ +"""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 names + + +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_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() From ca366d9c2048409dbc4b9da1213b0c39980a3713 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:31:47 -0500 Subject: [PATCH 2/2] Keep the NaN meanings classes document (Glide, damping, interval); plant CtorNan before the base cleans --- lib/audioeffects/_component.py | 11 ++++++++++- lib/audioeffects/convolutionreverb.py | 2 ++ lib/audioeffects/digitaldelay.py | 2 ++ lib/audioeffects/shimmerhall.py | 2 ++ lib/audioeffects/tapedelay.py | 2 ++ tests/support/kit_planted.py | 7 +++++-- tests/test_nan_options.py | 11 ++++++++++- 7 files changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/audioeffects/_component.py b/lib/audioeffects/_component.py index 5221dd12..e2befd1c 100644 --- a/lib/audioeffects/_component.py +++ b/lib/audioeffects/_component.py @@ -623,9 +623,14 @@ def _without_nan(cls, options, keywords): 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: @@ -633,7 +638,7 @@ def _without_nan(cls, options, keywords): keywords = dict(zip(names, options), **keywords) options = () return options, {name: value for name, value in keywords.items() - if not _is_nan(value)} + if name in own or not _is_nan(value)} class MetadataError(ValueError): @@ -823,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 diff --git a/lib/audioeffects/convolutionreverb.py b/lib/audioeffects/convolutionreverb.py index 5445525b..1cdc6e80 100644 --- a/lib/audioeffects/convolutionreverb.py +++ b/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' diff --git a/lib/audioeffects/digitaldelay.py b/lib/audioeffects/digitaldelay.py index 3977861f..373456aa 100644 --- a/lib/audioeffects/digitaldelay.py +++ b/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' diff --git a/lib/audioeffects/shimmerhall.py b/lib/audioeffects/shimmerhall.py index 08a3a76d..4870bd11 100644 --- a/lib/audioeffects/shimmerhall.py +++ b/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' diff --git a/lib/audioeffects/tapedelay.py b/lib/audioeffects/tapedelay.py index 5abc7d10..8bc8f022 100644 --- a/lib/audioeffects/tapedelay.py +++ b/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' diff --git a/tests/support/kit_planted.py b/tests/support/kit_planted.py index c88fef51..a2329ed9 100644 --- a/tests/support/kit_planted.py +++ b/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: diff --git a/tests/test_nan_options.py b/tests/test_nan_options.py index 94f45bdc..5ae39659 100644 --- a/tests/test_nan_options.py +++ b/tests/test_nan_options.py @@ -52,7 +52,7 @@ def options_of(cls): continue if p.default is None or isinstance(p.default, (int, float)): names.append(p.name) - return names + return [n for n in names if n not in cls._NAN_OWN] class ANanOptionIsTheOptionLeftOut(unittest.TestCase): @@ -76,6 +76,15 @@ def test_a_positional_nan_is_the_option_left_out(self): 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)),