Skip to content

Fix the error band, and let a caller read back whether it is drawn - #7

Merged
cboulay merged 2 commits into
mainfrom
fix/trace-grid-show-error
Aug 6, 2026
Merged

Fix the error band, and let a caller read back whether it is drawn#7
cboulay merged 2 commits into
mainfrom
fix/trace-grid-show-error

Conversation

@cboulay

@cboulay cboulay commented Aug 6, 2026

Copy link
Copy Markdown
Member

Two small things on top of v0.9.0, found by driving the trace grid against live
hardware.

The band raised on every frame

Turning on show_error made _curve_positions pass the band -- the lower and
upper edges stacked, 2 x n_ch rows -- to _map_y, which maps amplitudes into
per-channel cells and so expects n_ch:

mean, (n_ch, m)     -> (4, 10)
band, (2*n_ch, m)   -> ValueError: operands could not be broadcast together
                       with shapes (8,10) (4,1)

Each per-channel block is now mapped on its own.

It surfaced as a warning that points nowhere near it:

UserWarning: Could not resolve argspec of Figure animation function ...
calling it without arguments.

fastplotlib wraps both the argspec check and the call to an animation function
in one try, catching ValueError and TypeError. An exception raised inside
the callback is therefore reported as a failure to introspect it, and the
callback is called a second time. getfullargspec on the lambda works fine --
the message is simply misleading about its own cause, which is worth knowing for
anyone else who sees it.

The tests could not have caught it

make_widget stubbed _map_y with the identity -- precisely the collaborator
whose contract the band violated. One of those tests asserts the band has
2 x n_ch rows: the exact shape that raises, checked against a stub that
cannot. The tests now build the real _map_y, and the two that read raw
amplitudes off drawn y values compare through the mapping instead. Three fail
with the fix reverted.

show_error can be read back

set_show_error had no counterpart, so a caller could ask for the band but not
find out whether it got one. It reports the resolved value rather than the
request: asking for a band with statistics switched off resolves to off, and a
caller persisting the request would restore a setting that never took effect.

127 tests.

🤖 Generated with Claude Code

cboulay added 2 commits August 6, 2026 16:41
set_show_error had no counterpart, so a caller could ask for the band but not
find out whether it got one. It reads back the resolved value rather than the
request: asking for a band with statistics switched off resolves to off, and a
caller persisting the request would restore a setting that never took effect.
Ticking Mean +/- SD raised ValueError on every rendered frame. _map_y maps
amplitudes into per-channel cells, so it wants n_ch rows; the band is the lower
and upper edges stacked, 2 x n_ch, and the per-channel terms could not broadcast
against it. _curve_positions now maps each per-channel block on its own.

It surfaced as a warning that says nothing about any of this:

    UserWarning: Could not resolve argspec of Figure animation function ...
    calling it without arguments.

fastplotlib wraps both the argspec check and the call to the animation function
in one try, catching ValueError and TypeError. An exception from inside our
callback is therefore reported as a problem introspecting it, and the callback
is then called a second time. Nothing about the message suggests our code, and
nothing about it suggests the band.

The tests did not catch it because make_widget stubbed _map_y with the identity,
which is precisely the collaborator whose contract the band violated -- and one
of the tests asserts the band has 2 x n_ch rows, the exact shape that breaks it.
They now build the real _map_y, and two tests that read raw amplitudes off drawn
y values compare through the mapping instead. Three fail with the fix reverted.
@cboulay
cboulay merged commit f25dde3 into main Aug 6, 2026
11 checks passed
@cboulay
cboulay deleted the fix/trace-grid-show-error branch August 6, 2026 23:50
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