Skip to content

Give the parameterless math Units a usable SETTINGS type - #230

Merged
cboulay merged 1 commit into
devfrom
fix/parameterless-unit-settings
Aug 25, 2026
Merged

Give the parameterless math Units a usable SETTINGS type#230
cboulay merged 1 commit into
devfrom
fix/parameterless-unit-settings

Conversation

@cboulay

@cboulay cboulay commented Aug 25, 2026

Copy link
Copy Markdown
Member

The bug

Anscombe declares no SETTINGS — the source carries ... # SETTINGS = None, a comment rather than code. ezmsg's Unit metaclass therefore substitutes a bare ez.Settings(). But AnscombeTransformer is parameterized as BaseTransformer[None, ...], so its settings type resolves to NoneType, and _unify_settings fails its isinstance(settings, settings_type) assert:

>>> Anscombe().create_processor()
AssertionError: Settings must be of type <class 'NoneType'>

This surfaces at create_processor() — i.e. at initialize(), inside a running pipeline.

SETTINGS = None cannot be written literally: the metaclass requires an ez.Settings subclass and raises TypeError: issubclass() arg 1 must be a class at class-creation time. That is presumably why it was left as a comment.

Scope note

Abs and Invert have the byte-identical defect and fail with the same AssertionError. They're fixed here too — a patch that repaired one of three identical breakages seemed worse than useless. Happy to split them out if you'd rather keep this to Anscombe.

The fix

Each parameterless transform gets an empty ez.Settings subclass, and its transformer is parameterized with that instead of None:

class AnscombeSettings(ez.Settings):
    """The forward transform takes no parameters. ..."""

class AnscombeTransformer(BaseTransformer[AnscombeSettings, AxisArray, AxisArray]): ...

class Anscombe(BaseTransformerUnit[AnscombeSettings, AxisArray, AxisArray, AnscombeTransformer]):
    SETTINGS = AnscombeSettings

abs.py already had a vestigial class AbsSettings: pass — neither an ez.Settings subclass nor referenced anywhere. It becomes the real one.

The only visible behavior change is that AnscombeTransformer().settings is now AnscombeSettings() rather than None. All existing call sites construct these transformers with no arguments, so nothing else moves.

Test

A sweep over every Unit in ezmsg.sigproc.math (10 of them) asserting its SETTINGS is accepted by its paired transformer, and building the processor to exercise the real failure path. Reverting the source fix reproduces exactly:

AssertionError: Units with unusable SETTINGS:
    Abs: transformer is parameterized with None, needs an ez.Settings
    Anscombe: transformer is parameterized with None, needs an ez.Settings
    Invert: transformer is parameterized with None, needs an ez.Settings

Full suite: 4120 passed, 6 skipped.

Heads-up on an unrelated fragile test

The sweep deliberately runs at call time, not collection time. My first version parametrized over the units, which imported the whole math package during collection — and that tipped test_asarray.py::test_mlx_cache_limit_actually_bounds_the_cache into failing.

That test is order-sensitive on dev today, independent of this PR:

$ git stash              # clean dev
$ python -c "import ...all ezmsg.sigproc.math submodules...; pytest.main(['tests/unit/test_asarray.py'])"
FAILED test_asarray.py::test_mlx_cache_limit_actually_bounds_the_cache

It asserts mx.get_cache_memory() <= 32 MB, but MLX's cache limit is a soft target and the observed value sits right at the edge — I measured 17.00 MB and 34.30 MB on identical back-to-back trials in one process. Anything that shifts allocation history flips it. Not touched here, but it's a latent CI flake worth a separate look.

Anscombe, Abs and Invert declared no SETTINGS -- the source carried
``...  # SETTINGS = None``, a comment rather than code. ezmsg's Unit metaclass
therefore substituted a bare ``ez.Settings()``, which the paired transformer
then rejected: its settings type resolves to ``NoneType`` via the ``[None, ...]``
typevar, so ``_unify_settings`` fails its ``isinstance(settings, settings_type)``
assert. The break surfaces at ``create_processor()``, i.e. at ``initialize()``
inside a running pipeline:

    AssertionError: Settings must be of type <class 'NoneType'>

``SETTINGS = None`` cannot be written literally -- the metaclass requires an
``ez.Settings`` subclass and raises ``TypeError: issubclass() arg 1 must be a
class`` at class creation -- which is presumably why it was left commented out.
So each transform gets an empty ``ez.Settings`` subclass instead, and the
transformer is parameterized with that rather than ``None``.

abs.py already had a vestigial ``class AbsSettings: pass`` that was neither an
ez.Settings subclass nor referenced anywhere; it becomes the real one.

Adds a sweep over every Unit in ezmsg.sigproc.math asserting its SETTINGS is
accepted by its transformer, and building the processor to exercise the actual
failure path. The sweep runs at call time rather than collection time: importing
the whole math package during collection perturbs import order enough to tip
test_mlx_cache_limit_actually_bounds_the_cache, which is sensitive to
allocation history.
@cboulay
cboulay merged commit 3807028 into dev Aug 25, 2026
29 of 38 checks passed
@cboulay
cboulay deleted the fix/parameterless-unit-settings branch August 25, 2026 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant