Skip to content

Commit 1852b76

Browse files
SimonHeybrockclaude
andcommitted
Simplify overlay unit validation with CanvasSpec
Replace OverlayUnitKey (lossy string-based, weak cross-layer check) with CanvasSpec — a frozen dataclass holding coord_units and value_unit as native sc.Unit values. One validation function (validate_overlay_units) now serves both intra-layer and cross-layer sites with full consistency checks. ROI and annotation plotters opt out via participates_in_overlay_validation class flag, eliminating three _extract_overlay_unit_key overrides. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 435857a commit 1852b76

7 files changed

Lines changed: 133 additions & 342 deletions

File tree

src/ess/livedata/dashboard/plots.py

Lines changed: 52 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import weakref
88
from collections.abc import Callable
99
from dataclasses import dataclass
10+
from dataclasses import field as dataclass_field
1011
from typing import Any, ClassVar, cast
1112

1213
import holoviews as hv
@@ -216,117 +217,66 @@ def _compute_time_info(data: dict[str, sc.DataArray]) -> str | None:
216217

217218

218219
@dataclass(frozen=True)
219-
class OverlayUnitKey:
220-
"""Unit signature for cross-layer overlay validation.
220+
class CanvasSpec:
221+
"""Unit metadata extracted from a scipp DataArray for overlay validation.
221222
222-
Captures the units of all plotting dimensions and (optionally) the value
223-
dimension. Two keys are compatible when every shared kdim label has the
224-
same unit string and, if both keys carry a vdim unit, those match too.
223+
Captures coordinate units (keyed by dimension name) and the value unit.
224+
Two overlays are compatible when they share the same coordinate dimensions
225+
with matching units and matching value units.
225226
"""
226227

227-
kdim_units: tuple[tuple[str, str | None], ...] = ()
228-
vdim_unit: str | None = None
228+
coord_units: dict[str, sc.Unit] = dataclass_field(default_factory=dict)
229+
value_unit: sc.Unit = dataclass_field(default=sc.units.dimensionless)
229230

230231
@classmethod
231-
def from_data_array(
232-
cls,
233-
da: sc.DataArray,
234-
*,
235-
include_vdim: bool = True,
236-
) -> OverlayUnitKey:
237-
"""Build a key from a scipp DataArray using its dimension coordinates."""
238-
kdim_units = tuple(
239-
sorted(
240-
(dim, _unit_str(da.coords[dim].unit))
241-
for dim in da.dims
242-
if dim in da.coords
243-
)
232+
def from_data_array(cls, da: sc.DataArray) -> CanvasSpec:
233+
"""Build from a scipp DataArray using its dimension coordinates."""
234+
return cls(
235+
coord_units={
236+
dim: da.coords[dim].unit for dim in da.dims if dim in da.coords
237+
},
238+
value_unit=da.unit,
244239
)
245-
vdim_unit = _unit_str(da.unit) if include_vdim else None
246-
return cls(kdim_units=kdim_units, vdim_unit=vdim_unit)
247-
248-
249-
def _unit_str(unit: sc.Unit | None) -> str | None:
250-
"""Convert a scipp unit to its string representation, or None."""
251-
return str(unit) if unit is not None else None
252-
253-
254-
def _validate_overlay_units(data: dict[ResultKey, sc.DataArray]) -> None:
255-
"""Validate that all DataArrays in an overlay share compatible units.
256-
257-
Raises
258-
------
259-
ValueError
260-
If dimension names, coordinate units, or value units do not match.
261-
"""
262-
if len(data) < 2:
263-
return
264-
265-
items = iter(data.items())
266-
ref_key, ref = next(items)
267-
ref_source = ref_key.job_id.source_name
268-
269-
for key, da in items:
270-
source = key.job_id.source_name
271-
if ref.dims != da.dims:
272-
raise ValueError(
273-
f"Cannot overlay '{ref_source}' and '{source}': "
274-
f"dimension mismatch ({ref.dims} vs {da.dims})"
275-
)
276-
for dim in ref.dims:
277-
if dim in ref.coords and dim in da.coords:
278-
u_ref = ref.coords[dim].unit
279-
u_da = da.coords[dim].unit
280-
if u_ref != u_da:
281-
raise ValueError(
282-
f"Cannot overlay '{ref_source}' and '{source}': "
283-
f"unit mismatch for coordinate '{dim}' "
284-
f"({u_ref} vs {u_da})"
285-
)
286-
if ref.unit != da.unit:
287-
raise ValueError(
288-
f"Cannot overlay '{ref_source}' and '{source}': "
289-
f"value unit mismatch ({ref.unit} vs {da.unit})"
290-
)
291240

292241

293-
def validate_cross_layer_units(
294-
keys: list[tuple[str, OverlayUnitKey]],
242+
def validate_canvas_spec(
243+
entries: list[tuple[str, CanvasSpec]],
295244
) -> str | None:
296-
"""Check overlay unit keys from multiple layers for compatibility.
245+
"""Check that overlay entries share compatible units.
297246
298247
Parameters
299248
----------
300-
keys:
301-
Pairs of (layer label, unit key) for each data-carrying layer.
249+
entries:
250+
Pairs of (label, units) for each data-carrying element or layer.
302251
303252
Returns
304253
-------
305254
:
306255
An error message if units are incompatible, or None if compatible.
307256
"""
308-
if len(keys) < 2:
257+
if len(entries) < 2:
309258
return None
310259

311-
ref_label, ref_key = keys[0]
312-
ref_kdims = dict(ref_key.kdim_units)
313-
314-
for label, key in keys[1:]:
315-
other_kdims = dict(key.kdim_units)
316-
for dim_label, ref_unit in ref_kdims.items():
317-
if dim_label in other_kdims and ref_unit != other_kdims[dim_label]:
318-
return (
319-
f"Cannot overlay layers '{ref_label}' and '{label}': "
320-
f"unit mismatch for coordinate '{dim_label}' "
321-
f"({ref_unit} vs {other_kdims[dim_label]})"
322-
)
323-
if ref_key.vdim_unit is not None and key.vdim_unit is not None:
324-
if ref_key.vdim_unit != key.vdim_unit:
260+
ref_label, ref = entries[0]
261+
for label, other in entries[1:]:
262+
if ref.coord_units.keys() != other.coord_units.keys():
263+
return (
264+
f"Cannot overlay '{ref_label}' and '{label}': "
265+
f"dimension mismatch "
266+
f"({set(ref.coord_units)} vs {set(other.coord_units)})"
267+
)
268+
for dim, ref_unit in ref.coord_units.items():
269+
if ref_unit != other.coord_units[dim]:
325270
return (
326-
f"Cannot overlay layers '{ref_label}' and '{label}': "
327-
f"value unit mismatch "
328-
f"({ref_key.vdim_unit} vs {key.vdim_unit})"
271+
f"Cannot overlay '{ref_label}' and '{label}': "
272+
f"unit mismatch for coordinate '{dim}' "
273+
f"({ref_unit} vs {other.coord_units[dim]})"
329274
)
275+
if ref.value_unit != other.value_unit:
276+
return (
277+
f"Cannot overlay '{ref_label}' and '{label}': "
278+
f"value unit mismatch ({ref.value_unit} vs {other.value_unit})"
279+
)
330280
return None
331281

332282

@@ -338,6 +288,8 @@ class Plotter:
338288
This enables efficient polling-based update detection.
339289
"""
340290

291+
participates_in_overlay_validation: ClassVar[bool] = True
292+
341293
def __init__(
342294
self,
343295
*,
@@ -361,7 +313,7 @@ def __init__(
361313
"""
362314
self._normalize_to_rate = normalize_to_rate
363315
self._cached_state: Any | None = None
364-
self._overlay_unit_key: OverlayUnitKey | None = None
316+
self.canvas_spec: CanvasSpec | None = None
365317
self._presenters: weakref.WeakSet[PresenterBase] = weakref.WeakSet()
366318
self.autoscaler_kwargs = kwargs
367319
self.autoscalers: dict[ResultKey, Autoscaler] = {}
@@ -374,25 +326,6 @@ def __init__(
374326
# with responsive mode in Panel containers (upstream bug).
375327
self._sizing_opts: dict[str, Any] = {'responsive': True}
376328

377-
@property
378-
def overlay_unit_key(self) -> OverlayUnitKey | None:
379-
"""Unit signature from the last compute(), for cross-layer validation."""
380-
return self._overlay_unit_key
381-
382-
def _extract_overlay_unit_key(
383-
self, data: dict[ResultKey, sc.DataArray]
384-
) -> OverlayUnitKey | None:
385-
"""Extract an overlay unit key from input data.
386-
387-
Default implementation uses dimension coordinates and value unit from the
388-
first DataArray. Subclasses with non-standard coordinate structures (e.g.,
389-
ROI plotters) should override this method.
390-
"""
391-
if not data:
392-
return None
393-
da = next(iter(data.values()))
394-
return OverlayUnitKey.from_data_array(da)
395-
396329
@staticmethod
397330
def _make_tick_opts(tick_params: TickParams | None) -> dict[str, Any]:
398331
"""
@@ -566,13 +499,19 @@ def compute(
566499
if self._normalize_to_rate:
567500
data = {key: _normalize_to_rate(da) for key, da in data.items()}
568501

569-
self._overlay_unit_key = self._extract_overlay_unit_key(data)
570-
571502
resolver = title_resolver or TitleResolver()
572503
plots: list[hv.Element] = []
573504
try:
574-
if self.layout_params.combine_mode == 'overlay':
575-
_validate_overlay_units(data)
505+
if self.participates_in_overlay_validation and data:
506+
entries = [
507+
(key.job_id.source_name, CanvasSpec.from_data_array(da))
508+
for key, da in data.items()
509+
]
510+
if self.layout_params.combine_mode == 'overlay':
511+
error = validate_canvas_spec(entries)
512+
if error is not None:
513+
raise ValueError(error)
514+
self.canvas_spec = entries[0][1]
576515
for data_key, da in data.items():
577516
label = resolver.get_legend_label(
578517
data_key.job_id.source_name, data_key.output_name

src/ess/livedata/dashboard/roi_readback_plots.py

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,23 +21,7 @@
2121
)
2222
from ess.livedata.config.workflow_spec import ResultKey
2323

24-
from .plots import OverlayUnitKey, Plotter, _unit_str
25-
26-
27-
def _roi_overlay_unit_key(da: sc.DataArray) -> OverlayUnitKey:
28-
"""Build an overlay unit key from ROI coordinate metadata.
29-
30-
ROI DataArrays use named coords ('x', 'y') rather than dimension
31-
coordinates, and have no meaningful value unit.
32-
"""
33-
kdim_units = tuple(
34-
sorted(
35-
(name, _unit_str(da.coords[name].unit))
36-
for name in ('x', 'y')
37-
if name in da.coords
38-
)
39-
)
40-
return OverlayUnitKey(kdim_units=kdim_units, vdim_unit=None)
24+
from .plots import Plotter
4125

4226

4327
class ROIReadbackStyle(pydantic.BaseModel):
@@ -87,6 +71,8 @@ class RectanglesReadbackPlotter(Plotter):
8771
rectangles with per-shape colors based on the ROI index.
8872
"""
8973

74+
participates_in_overlay_validation = False
75+
9076
def __init__(self, params: RectanglesReadbackParams) -> None:
9177
super().__init__()
9278
self._params = params
@@ -97,14 +83,6 @@ def from_params(cls, params: RectanglesReadbackParams) -> RectanglesReadbackPlot
9783
"""Create plotter from params."""
9884
return cls(params)
9985

100-
def _extract_overlay_unit_key(
101-
self, data: dict[ResultKey, sc.DataArray]
102-
) -> OverlayUnitKey | None:
103-
if not data:
104-
return None
105-
da = next(iter(data.values()))
106-
return _roi_overlay_unit_key(da)
107-
10886
def plot(
10987
self, data: sc.DataArray, data_key: ResultKey, *, label: str = '', **kwargs
11088
) -> hv.Rectangles:
@@ -201,6 +179,8 @@ class PolygonsReadbackPlotter(Plotter):
201179
polygons with per-shape colors based on the ROI index.
202180
"""
203181

182+
participates_in_overlay_validation = False
183+
204184
def __init__(self, params: PolygonsReadbackParams) -> None:
205185
super().__init__()
206186
self._params = params
@@ -211,14 +191,6 @@ def from_params(cls, params: PolygonsReadbackParams) -> PolygonsReadbackPlotter:
211191
"""Create plotter from params."""
212192
return cls(params)
213193

214-
def _extract_overlay_unit_key(
215-
self, data: dict[ResultKey, sc.DataArray]
216-
) -> OverlayUnitKey | None:
217-
if not data:
218-
return None
219-
da = next(iter(data.values()))
220-
return _roi_overlay_unit_key(da)
221-
222194
def plot(
223195
self, data: sc.DataArray, data_key: ResultKey, *, label: str = '', **kwargs
224196
) -> hv.Polygons:

src/ess/livedata/dashboard/roi_request_plots.py

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
get_roi_mapper,
4949
)
5050

51-
from .plots import OverlayUnitKey, Plotter, PresenterBase
51+
from .plots import Plotter, PresenterBase
5252
from .static_plots import Color, LineDash, RectanglesCoordinates
5353

5454
if TYPE_CHECKING:
@@ -533,6 +533,8 @@ class BaseROIRequestPlotter(Plotter, ABC, Generic[ROIType, ParamsType, Converter
533533
skip logic, publishing) is handled via a closure-based edit callback.
534534
"""
535535

536+
participates_in_overlay_validation = False
537+
536538
def __init__(
537539
self,
538540
params: ParamsType,
@@ -626,16 +628,6 @@ def compute(
626628
else None
627629
)
628630

629-
# Build overlay unit key from ROI coordinate units
630-
kdim_units = tuple(
631-
sorted(
632-
(name, unit)
633-
for name, unit in [('x', self._x_unit), ('y', self._y_unit)]
634-
if unit is not None
635-
)
636-
)
637-
self._overlay_unit_key = OverlayUnitKey(kdim_units=kdim_units, vdim_unit=None)
638-
639631
# Forward data (presenter may use in future)
640632
self._set_cached_state(data)
641633
return data

src/ess/livedata/dashboard/widgets/plot_grid_tabs.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
SubscriptionId,
3636
)
3737
from ..plot_params import PlotAspectType, StretchMode
38-
from ..plots import OverlayUnitKey, validate_cross_layer_units
38+
from ..plots import CanvasSpec, validate_canvas_spec
3939
from ..save_filename import build_save_filename_from_cell, make_save_filename_hook
4040
from ..session_layer import SessionLayer
4141
from ..session_updater import SessionUpdater
@@ -941,7 +941,7 @@ def _get_session_composed_plot(
941941
Composed plot from session DMaps/elements, or None if none available.
942942
"""
943943
plots = []
944-
unit_keys: list[tuple[str, OverlayUnitKey]] = []
944+
overlay_entries: list[tuple[str, CanvasSpec]] = []
945945
has_layout = False
946946
for layer in cell.layers:
947947
layer_id = layer.layer_id
@@ -969,17 +969,18 @@ def _get_session_composed_plot(
969969
has_layout = True
970970
plots.append(dmap)
971971

972-
# Collect unit key for cross-layer validation
972+
# Collect overlay units for cross-layer validation
973+
# (only data-carrying plotters set canvas_spec)
973974
if state is not None and state.plotter is not None:
974-
key = state.plotter.overlay_unit_key
975-
if key is not None:
976-
unit_keys.append((layer.config.plot_name, key))
975+
units = state.plotter.canvas_spec
976+
if units is not None:
977+
overlay_entries.append((layer.config.plot_name, units))
977978

978979
if not plots:
979980
return None
980981

981982
# Validate cross-layer unit compatibility before composing
982-
error = validate_cross_layer_units(unit_keys)
983+
error = validate_canvas_spec(overlay_entries)
983984
if error is not None:
984985
return hv.Text(0.5, 0.5, f"Error: {error}").opts(
985986
text_align='center', text_baseline='middle'

0 commit comments

Comments
 (0)