From 8c2ffc1c76f23de297c911b7f080429517ac15ce Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Thu, 6 Aug 2026 16:41:21 -0400 Subject: [PATCH 1/2] Let a caller read back whether the error band is drawn 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. --- src/phosphor/trace_grid.py | 9 +++++++++ tests/test_trace_grid.py | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/phosphor/trace_grid.py b/src/phosphor/trace_grid.py index 654ef0c..5d4635f 100644 --- a/src/phosphor/trace_grid.py +++ b/src/phosphor/trace_grid.py @@ -338,6 +338,15 @@ def show_individual(self) -> bool: def show_mean(self) -> bool: return self._show_mean + @property + def show_error(self) -> bool: + """Whether the standard-deviation band is drawn. + + Reads back the resolved value, not what was asked for: requesting a band + without statistics resolves to off, because there is no spread to draw. + """ + return self._show_error + @property def autoscale(self) -> bool: return self._autoscale diff --git a/tests/test_trace_grid.py b/tests/test_trace_grid.py index 91a5324..03b58ee 100644 --- a/tests/test_trace_grid.py +++ b/tests/test_trace_grid.py @@ -316,3 +316,14 @@ def test_clearing_and_refilling_brings_the_graphics_back(): w._buffer.push(wave(2.0)) w._refresh_lines() assert w._indiv_ml is not None + + +def test_show_error_reads_back_what_was_resolved_not_what_was_asked(): + """A band without statistics is not a band. Callers persist this value, so + reading back the request rather than the resolution would restore a setting + that never took effect.""" + w = make_widget(show_error=True, track_statistics=False) + assert w.show_error is False + + w = make_widget(show_error=True) + assert w.show_error is True From 812d4f31a3ff6585155d37d3b531694e61606aa8 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Thu, 6 Aug 2026 19:40:39 -0400 Subject: [PATCH 2/2] Map the error band per channel, not across twice as many rows 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. --- src/phosphor/trace_grid.py | 15 +++++++++++---- tests/test_trace_grid.py | 14 ++++++++++---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/phosphor/trace_grid.py b/src/phosphor/trace_grid.py index 5d4635f..2804ff3 100644 --- a/src/phosphor/trace_grid.py +++ b/src/phosphor/trace_grid.py @@ -590,13 +590,20 @@ def _summary_positions(self) -> tuple[np.ndarray | None, np.ndarray | None]: return mean_pos, self._curve_positions(minmax_decimate(band, self._dec_plan)) def _curve_positions(self, curve: np.ndarray) -> np.ndarray: - """One line per row of *curve*, laid into the cells.""" + """One line per row of *curve*, laid into the cells. + + *curve* holds a whole number of per-channel blocks -- one for a mean, + two for the lower and upper edges of a band. Each block is mapped on its + own, because the cell mapping is per channel and would otherwise be + asked to broadcast a block of channels against twice as many rows. + """ n_lines, m = curve.shape[0], curve.shape[-1] - pos = np.empty((n_lines, m, 3), dtype=np.float32) - # The band is two curves per channel, so x tiles rather than broadcasts. reps = n_lines // self._n_ch + pos = np.empty((n_lines, m, 3), dtype=np.float32) pos[..., 0] = np.tile(self._x_line_dec, (reps, 1)) - pos[..., 1] = self._map_y(curve) + for i in range(reps): + block = slice(i * self._n_ch, (i + 1) * self._n_ch) + pos[block, :, 1] = self._map_y(curve[block]) pos[..., 2] = 0.0 return pos diff --git a/tests/test_trace_grid.py b/tests/test_trace_grid.py index 03b58ee..bb14372 100644 --- a/tests/test_trace_grid.py +++ b/tests/test_trace_grid.py @@ -33,8 +33,11 @@ def make_widget(n_ch=2, n_samples=4, history=3, **config_kwargs) -> TraceGridWid w._x_line_dec = np.tile(np.arange(n_samples, dtype=np.float32), (n_ch, 1)) w._indiv_ml = w._mean_ml = w._error_ml = None w._graphics_version = -1 - # _map_y is affine per channel; identity keeps these tests about layout. - w._map_y = lambda a: np.asarray(a, dtype=np.float32) + # The real _map_y, not a stub. It is per channel, and stubbing it is what + # let a band of 2 x n_ch rows reach it and raise on every frame while the + # tests stayed green. + w._rects = np.column_stack([np.zeros(n_ch), np.arange(n_ch, dtype=float), np.ones(n_ch)]) + w._y_min, w._y_max = -1000.0, 1000.0 return w @@ -196,7 +199,9 @@ def test_the_mean_spans_more_waveforms_than_are_drawn(): w._buffer.push(wave(v, n_ch=1)) mean_pos, _ = w._summary_positions() - np.testing.assert_allclose(mean_pos[0, :, 1], 3.0) # mean of 1..5, not of 4..5 + # Compared through the cell mapping, since that is what the drawn y is. + expected = w._map_y(np.full((1, w._n_samples), 3.0, dtype=np.float32)) # 1..5, not 4..5 + np.testing.assert_allclose(mean_pos[0, :, 1], expected[0], rtol=1e-6) # ---- graphics actually get created ------------------------------------------ @@ -286,7 +291,8 @@ def test_the_newest_waveform_reaches_the_graphic(): w._refresh_lines() ys = w._indiv_ml.data[..., 1] - assert np.isclose(ys, 7.0).any(), "the value just pushed should be in the graphic" + expected = w._map_y(np.full((w._n_ch, w._n_samples), 7.0, dtype=np.float32))[0, 0] + assert np.isclose(ys, expected).any(), "the value just pushed should be in the graphic" def test_the_error_band_appears_once_there_is_a_spread():