toolchain/mfc/viz/interactive.py repeats three small, self-contained pieces of logic that are pure functions and trivially extractable. Dedup'ing them removes a live copy-paste hazard (change one copy, forget the others) and is a good opportunity to add the first automated tests for this module.
This is the low-risk subset carved out of the larger _update decomposition (tracked separately) — it touches no callback control flow and is unit-testable.
1. Status html.Div built 4× verbatim
Lines 1971, 2120, 2470, 2626 are the same 8-child html.Div (step {step} / · shape {raw.shape} / Br / min+value / max+value, same _YELLOW/_MUTED/_BLUE/_RED styles), differing only in the min/max local names.
→ add _status_div(step, shape, dmin, dmax) -> html.Div and call it at all four sites.
2. Log/identity transform closure _tf redefined 4×
Lines 379, 386 (inside _prefetch_3d_mesh) and 1853, 1861 (inside _update):
def _tf(arr):
return np.where(arr > 0, np.log10(np.maximum(arr, 1e-300)), np.nan)
# else
def _tf(arr):
return arr
→ add _log_transform(log: bool) returning the transform (and the cmin/cmax pair, since sites 379/386 inline-compute those right next to the closure — extract them together so the cmin/cmax duplication doesn't get left behind). The 1e-300 floor should become a module constant _LOG_FLOOR at the same time.
3. 11-arg 3D mesh cache-key tuple repeated
The literal (step, var, mode, log, vmin_in, vmax_in, iso_min_frac, iso_max_frac, iso_n, vol_min_frac, vol_max_frac) appears at 350 (_prefetch_3d_mesh) and 432 (_get_cached_3d_mesh), and is passed positionally from _update at ~1890.
→ add _mesh3_key(...) -> tuple (or a small namedtuple) and use it at all three sites.
Tests
toolchain/mfc/viz/test_viz.py currently has no coverage of interactive.py at all. Add a few unit tests for _status_div, _log_transform, and _mesh3_key — these would be the first tests for this module and make the dedup verifiable.
Behavior preservation / risk: all three are pure-function extractions with no control-flow change; toolchain-only, no golden-file impact.
Filed from a repo-wide code-cleanliness review; verified against master @ 40dde5e.
Code references
toolchain/mfc/viz/interactive.pyrepeats three small, self-contained pieces of logic that are pure functions and trivially extractable. Dedup'ing them removes a live copy-paste hazard (change one copy, forget the others) and is a good opportunity to add the first automated tests for this module.This is the low-risk subset carved out of the larger
_updatedecomposition (tracked separately) — it touches no callback control flow and is unit-testable.1. Status
html.Divbuilt 4× verbatimLines 1971, 2120, 2470, 2626 are the same 8-child
html.Div(step {step}/· shape {raw.shape}/Br/min+value /max+value, same_YELLOW/_MUTED/_BLUE/_REDstyles), differing only in the min/max local names.→ add
_status_div(step, shape, dmin, dmax) -> html.Divand call it at all four sites.2. Log/identity transform closure
_tfredefined 4×Lines 379, 386 (inside
_prefetch_3d_mesh) and 1853, 1861 (inside_update):→ add
_log_transform(log: bool)returning the transform (and thecmin/cmaxpair, since sites 379/386 inline-compute those right next to the closure — extract them together so the cmin/cmax duplication doesn't get left behind). The1e-300floor should become a module constant_LOG_FLOORat the same time.3. 11-arg 3D mesh cache-key tuple repeated
The literal
(step, var, mode, log, vmin_in, vmax_in, iso_min_frac, iso_max_frac, iso_n, vol_min_frac, vol_max_frac)appears at 350 (_prefetch_3d_mesh) and 432 (_get_cached_3d_mesh), and is passed positionally from_updateat ~1890.→ add
_mesh3_key(...) -> tuple(or a small namedtuple) and use it at all three sites.Tests
toolchain/mfc/viz/test_viz.pycurrently has no coverage ofinteractive.pyat all. Add a few unit tests for_status_div,_log_transform, and_mesh3_key— these would be the first tests for this module and make the dedup verifiable.Behavior preservation / risk: all three are pure-function extractions with no control-flow change; toolchain-only, no golden-file impact.
Filed from a repo-wide code-cleanliness review; verified against
master@40dde5e.Code references
toolchain/mfc/viz/interactive.py:1971— status Div copy 1 of 4 (others: 2120, 2470, 2626)toolchain/mfc/viz/interactive.py:379-386—_tfclosures (also 1853, 1861)toolchain/mfc/viz/interactive.py:350— 11-arg cache key (also 432, ~1890)toolchain/mfc/viz/test_viz.py— test file (no interactive.py coverage yet)