From 38f19f9dafc779fe35be41d6e0b263e77927d5b2 Mon Sep 17 00:00:00 2001 From: Francois Drielsma Date: Wed, 21 Oct 2026 14:01:53 -0700 Subject: [PATCH] Improve Drawer layout overrides and responsive defaults --- docs/source/api/vis.rst | 40 ++++++++++++ src/spine/vis/drawer/out/drawer.py | 54 +++++++++++++---- src/spine/vis/layout/plotly.py | 65 +++++++++++++------- src/spine/vis/scene/plotly.py | 3 + test/test_vis/test_drawer/test_out.py | 77 ++++++++++++++++++++++++ test/test_vis/test_layout/test_layout.py | 63 ++++++++++++++++++- test/test_vis/test_scene/test_scene.py | 6 ++ 7 files changed, 273 insertions(+), 35 deletions(-) diff --git a/docs/source/api/vis.rst b/docs/source/api/vis.rst index 728b0ff8..d403ec35 100644 --- a/docs/source/api/vis.rst +++ b/docs/source/api/vis.rst @@ -22,6 +22,46 @@ Use this package to inspect detector point clouds, overlay reconstructed or trut scene trace +Figure Layout +------------- + +Pass layout defaults to :class:`Drawer`, then override them for individual +:meth:`Drawer.get` calls. Each call returns an independent figure; overrides +do not change the drawer defaults or earlier figures. + +.. code-block:: python + + drawer = Drawer(data, draw_mode="both", height=800, title="Event display") + fig = drawer.get( + "particles", + title="Selected event", + titles=["Reconstruction", "Truth"], + margin={"l": 20}, + ) + untitled = drawer.get("particles", title=None) + +``title`` labels the entire figure and accepts a string or a Plotly title +dictionary. ``Drawer.get(titles=...)`` labels the two split panels. In the +lower-level ``layout3d`` helper, ``titles`` instead labels the three axes; +constructor ``Drawer(..., titles=...)`` options are forwarded to that helper. + +Single and split views default to ``width=None``, ``autosize=True``, and +``height=800``. The automatic width fits the containing element, while a +concrete height keeps 3D views usable in containers without a defined height. +Explicit widths, heights, and ``autosize`` settings are preserved. Notebook or +web embedding must also allow Plotly to resize when its container changes. + +Nonempty figure titles reserve 60 pixels at the top. Split-panel titles reserve +another 42 pixels. Explicit margin edges win, including ``margin={"t": 0}``; +unspecified edges retain automatic defaults. A per-call margin dictionary +replaces the constructor margin dictionary rather than merging with it. +Other per-call options likewise replace constructor options by keyword. + +``dual_figure3d`` copies supplied layouts and preserves their dimensions and +margin edges unless explicitly overridden. Omitting its width or height keeps +the supplied value; passing ``None`` clears that dimension. Without a supplied +layout, it uses the same responsive width and 800-pixel height defaults. + Renderer-Neutral Scenes ----------------------- diff --git a/src/spine/vis/drawer/out/drawer.py b/src/spine/vis/drawer/out/drawer.py index b0c66f10..0e40351e 100644 --- a/src/spine/vis/drawer/out/drawer.py +++ b/src/spine/vis/drawer/out/drawer.py @@ -2,6 +2,7 @@ from __future__ import annotations +from copy import deepcopy from typing import Any import numpy as np @@ -298,9 +299,14 @@ def get_scene( matched_crthit_only=matched_crthit_only, ) - def build_geometry_traces(self) -> list[Any]: + def build_geometry_traces(self, dark: bool | None = None) -> list[Any]: """Build detector outlines with contrast for the configured theme. + Parameters + ---------- + dark : bool, optional + Per-figure theme override. If omitted, use the constructor theme. + Returns ------- list @@ -316,7 +322,9 @@ def build_geometry_traces(self) -> list[Any]: "A geometry drawer is required to build geometry traces." ) - dark = self.layout_kwargs.get("dark", False) + if dark is None: + dark = self.layout_kwargs.get("dark", False) + color = "rgba(255,255,255,0.400)" if dark else "rgba(0,0,0,0.200)" traces = self.geo_drawer.tpc_traces(meta=self.meta, color=color) for trace in traces: @@ -464,6 +472,7 @@ def get( synchronize: bool = False, titles: list[str] | None = None, split_traces: bool = False, + **kwargs: Any, ) -> go.Figure: """Draw the requested reconstructed and/or truth object family. @@ -503,17 +512,30 @@ def get( synchronize : bool, default False If ``True`` and two scenes are drawn, synchronize their cameras. titles : List[str], optional - Titles for the two scenes when ``split_scene`` is enabled. + Panel titles for the two scenes when ``split_scene`` is enabled. + Use ``title`` for a title above the entire figure. split_traces : bool, default False If ``True``, emit one trace per object instead of one shared trace. + **kwargs : Any + Per-figure options forwarded to :func:`spine.vis.layout.layout3d`. + These replace constructor options by keyword without changing + subsequent figures. For example, ``title=None`` clears an inherited + figure title, and ``width=None`` enables responsive width. Margin + edges supplied in this call replace the constructor margin option; + unspecified edges use automatic defaults. Returns ------- go.Figure Plotly figure containing all requested object and detector traces. """ + # Validate the requested object type and attributes, and build the traces for + # each truth/reconstruction prefix. attrs = self.validate_request(obj_type, attr) + # Isolate nested layout dictionaries from callers and later figures. + layout_kwargs = deepcopy({**self.layout_kwargs, **kwargs}) + traces: dict[str, list] = {} for prefix in self.prefixes: obj_name = f"{prefix}_{obj_type}" @@ -687,7 +709,9 @@ def get( # Draw the geometry overlay if a GeoDrawer is available if self.geo_drawer is not None: - geo_traces = self.build_geometry_traces() + geo_traces = self.build_geometry_traces( + dark=layout_kwargs.get("dark", False) + ) if self.prefixes and self.split_scene: for prefix in self.prefixes: traces[prefix] += geo_traces @@ -695,15 +719,16 @@ def get( traces[self.prefixes[-1]] += geo_traces # Build the layout and assemble the requested traces and geometry overlay - layout = layout3d( - geo=self.geo, - use_geo=self.geo is not None, - meta=self.meta, - detector_coords=self.detector_coords, - show_optical=show_optical, - show_crt=show_crt, - **self.layout_kwargs, - ) + layout_kwargs = { + "geo": self.geo, + "use_geo": self.geo is not None, + "meta": self.meta, + "detector_coords": self.detector_coords, + "show_optical": show_optical, + "show_crt": show_crt, + **layout_kwargs, + } + layout = layout3d(**layout_kwargs) # If both truth and reconstruction are drawn and split_scene is enabled, draw # them in separate scenes with linked cameras. Otherwise, draw all traces @@ -711,6 +736,9 @@ def get( if len(self.prefixes) > 1 and self.split_scene: if titles is None: titles = [f"Reconstructed {obj_type}", f"Truth {obj_type}"] + + # Let the split helper fill defaults around explicit margin edges. + layout.margin = layout_kwargs.get("margin") return dual_figure3d( traces["reco"], traces["truth"], diff --git a/src/spine/vis/layout/plotly.py b/src/spine/vis/layout/plotly.py index 02bb0382..5a6056bf 100644 --- a/src/spine/vis/layout/plotly.py +++ b/src/spine/vis/layout/plotly.py @@ -4,6 +4,7 @@ from collections.abc import Sequence from copy import deepcopy +from types import EllipsisType from typing import Any import numpy as np @@ -60,7 +61,7 @@ def layout3d( up_dir: np.ndarray | None = None, backgroundcolor: str = "white", gridcolor: str = "lightgray", - width: int = 800, + width: int | None = None, height: int = 800, showlegend: bool = True, camera: dict[str, Any] | None = None, @@ -104,8 +105,8 @@ def layout3d( Plot background color. gridcolor : str, default ``"lightgray"`` Axis grid color. - width : int, default 800 - Figure width in pixels. + width : int, optional + Figure width in pixels. If omitted, fit the containing element. height : int, default 800 Figure height in pixels. showlegend : bool, default True @@ -119,11 +120,14 @@ def layout3d( dark : bool, default False If ``True``, use a dark Plotly theme. margin : Dict[str, int], optional - Plot margins. + Plot margins. Explicit edges override the defaults, including a zero + top margin. Otherwise, reserve 60 pixels above a nonempty figure title. hoverlabel : Dict[str, Any], optional Hoverlabel style dictionary. **kwargs : Any - Additional keyword arguments forwarded to :class:`plotly.graph_objs.Layout`. + Additional keyword arguments forwarded to :class:`plotly.graph_objs.Layout`, + including the figure ``title`` (distinct from axis ``titles``). + ``autosize`` defaults to ``True``; explicit dimensions are preserved. Returns ------- @@ -227,8 +231,10 @@ def layout3d( kwargs["paper_bgcolor"] = "black" backgroundcolor = "black" - if margin is None: - margin = {"b": 0, "t": 0, "l": 0, "r": 0} + # Normalize titles through Plotly so strings and title dictionaries agree. + title = go.Layout(title=kwargs.get("title")).title + margin = {"b": 0, "t": 60 if title.text else 0, "l": 0, "r": 0, **(margin or {})} + kwargs.setdefault("autosize", True) if hoverlabel is None: hoverlabel = {"font_family": "Droid sans, monospace"} @@ -268,8 +274,8 @@ def dual_figure3d( traces_right: Sequence[object], layout: go.Layout | None = None, titles: list[str] | None = None, - width: int = 1500, - height: int = 750, + width: int | None | EllipsisType = ..., + height: int | None | EllipsisType = ..., synchronize: bool = False, margin: dict[str, int] | None = None, **kwargs: Any, @@ -283,17 +289,21 @@ def dual_figure3d( traces_right : Sequence[object] Plotly traces to draw in the right subplot. layout : go.Layout, optional - Base layout to apply to the figure. + Base layout to apply to the figure. It is copied, not modified. titles : List[str], optional Subplot titles for the left and right scenes. - width : int, default 1500 - Figure width in pixels. - height : int, default 750 - Figure height in pixels. + width : int or None, optional + Figure width in pixels. Omission preserves the base layout width; + explicit ``None`` clears it for responsive sizing. Without a base + layout, the default is ``None``. + height : int or None, optional + Figure height in pixels. Omission preserves the base layout height; + explicit ``None`` clears it. Without a base layout, the default is 800. synchronize : bool, default False If ``True``, keep the two subplot cameras synchronized. margin : Dict[str, int], optional - Figure margins. + Figure margins. Explicit edges override the base layout margins and + defaults. Defaults reserve space for panel and figure titles. **kwargs : Any Additional keyword arguments used only when a default layout must be created internally. @@ -321,14 +331,27 @@ def dual_figure3d( fig.add_traces(traces_left, rows=[1] * num_left, cols=[1] * num_left) fig.add_traces(traces_right, rows=[1] * num_right, cols=[2] * num_right) - if margin is None: - # Leave enough room between subplot titles and the containing panel. - margin = {"b": 20, "t": 42, "l": 20, "r": 20} - if layout is None: - layout = layout3d(width=width, height=height, margin=margin, **kwargs) + layout = layout3d(**kwargs) + base_margin = {} else: - layout.update({"width": width, "height": height, "margin": margin}) + layout = go.Layout(layout) + base_margin = layout.margin.to_plotly_json() + + # Omitted dimensions preserve the layout; None explicitly clears a size. + if width is not ...: + layout.width = width + if height is not ...: + layout.height = height + top = (42 if titles and any(titles) else 0) + (60 if layout.title.text else 0) + layout.margin = { + "b": 20, + "t": top, + "l": 20, + "r": 20, + **base_margin, + **(margin or {}), + } fig.layout.update(layout) if synchronize: diff --git a/src/spine/vis/scene/plotly.py b/src/spine/vis/scene/plotly.py index 80f356a2..37d88f21 100644 --- a/src/spine/vis/scene/plotly.py +++ b/src/spine/vis/scene/plotly.py @@ -74,6 +74,9 @@ def render( if metadata.get("up_dir") is not None: layout_kwargs.setdefault("up_dir", np.asarray(metadata["up_dir"])) layout = layout3d(**layout_kwargs) + if len(scene.views) == 2: + # Preserve caller margins while deferring split-title defaults. + layout.margin = layout_kwargs.get("margin") # Return a valid empty figure for scenes without views if not scene.views: diff --git a/test/test_vis/test_drawer/test_out.py b/test/test_vis/test_drawer/test_out.py index ed771d84..11c4a80d 100644 --- a/test/test_vis/test_drawer/test_out.py +++ b/test/test_vis/test_drawer/test_out.py @@ -1,5 +1,6 @@ """Tests for output object visualization.""" +from copy import deepcopy from importlib import import_module from types import SimpleNamespace @@ -32,6 +33,82 @@ def reset_geo_manager(): GeoManager.reset() +@pytest.mark.parametrize("split", [False, True]) +def test_drawer_layout_overrides_are_per_figure(split): + """Each get call resolves layout options without changing other figures.""" + data = { + "points": np.empty((0, 3)), + "points_label": np.empty((0, 3)), + "reco_particles": [], + "truth_particles": [], + } + constructor = { + "title": {"text": "Default"}, + "width": 900, + "height": 650, + "margin": {"l": 11}, + } + original = deepcopy(constructor) + drawer = Drawer(data, split_scene=split, **constructor) + first = drawer.get("particles") + first_layout = deepcopy(first.layout.to_plotly_json()) + overrides = { + "title": {"text": "Override"}, + "width": None, + "height": 550, + "margin": {"t": 0}, + "autosize": False, + } + override_copy = deepcopy(overrides) + second = drawer.get("particles", **overrides) + cleared = drawer.get("particles", title=None) + repeated = drawer.get("particles") + + assert first.layout.title.text == "Default" + assert (first.layout.width, first.layout.height) == (900, 650) + assert first.layout.margin.t == (102 if split else 60) + assert first.layout.margin.l == 11 + assert second.layout.title.text == "Override" + assert second.layout.width is None + assert second.layout.height == 550 + assert second.layout.autosize is False + assert second.layout.margin.t == 0 + assert cleared.layout.title.text is None + assert cleared.layout.margin.t == (42 if split else 0) + assert repeated.layout.to_plotly_json() == first_layout + assert first.layout.to_plotly_json() == first_layout + assert drawer.layout_kwargs == original + assert constructor == original + assert overrides == override_copy + + +@pytest.mark.parametrize("split", [False, True]) +def test_drawer_responsive_defaults_and_panel_titles(split): + """Single and split views share dimensions and distinguish title levels.""" + drawer = Drawer( + { + "points": np.empty((0, 3)), + "points_label": np.empty((0, 3)), + "reco_particles": [], + "truth_particles": [], + }, + split_scene=split, + titles=["X", "Y", "Z"], + ) + kwargs = {"titles": ["Left", "Right"]} if split else {} + figure = drawer.get("particles", title="Event", **kwargs) + assert figure.layout.width is None + assert figure.layout.height == 800 + assert figure.layout.autosize is True + assert figure.layout.title.text == "Event" + assert figure.layout.scene.xaxis.title.text == "X" + if split: + assert [annotation.text for annotation in figure.layout.annotations] == [ + "Left", + "Right", + ] + + def test_drawer_accepts_derived_hover_attr(): """Drawer validation should accept derived DataBase attributes.""" data = { diff --git a/test/test_vis/test_layout/test_layout.py b/test/test_vis/test_layout/test_layout.py index 8ed0d21a..dce21f70 100644 --- a/test/test_vis/test_layout/test_layout.py +++ b/test/test_vis/test_layout/test_layout.py @@ -1,5 +1,6 @@ """Tests for layout visualization helpers.""" +from copy import deepcopy from importlib import import_module from types import SimpleNamespace @@ -48,11 +49,71 @@ def test_layout_and_dual_figure_helpers(): assert layout.scene.xaxis.range == (0, 1) assert len(fig.data) == 2 - assert fig.layout.margin.to_plotly_json() == {"b": 20, "t": 42, "l": 20, "r": 20} + assert fig.layout.margin.to_plotly_json() == {"b": 20, "t": 0, "l": 20, "r": 20} assert width > height > 0 assert color_rgba((1, 2, 3), 0.5) == "rgba(1, 2, 3, 0.5)" +@pytest.mark.parametrize( + "title", [None, "", {}, {"text": ""}, "Event", {"text": "Event"}] +) +def test_layout_title_margins_and_responsive_defaults(title): + """Only nonempty figure titles reserve space, for either title syntax.""" + layout = layout3d(title=title) + assert layout.width is None + assert layout.height == 800 + assert layout.autosize is True + assert layout.margin.t == (60 if layout.title.text else 0) + + +@pytest.mark.parametrize("split", [False, True]) +def test_layout_explicit_sizes_and_partial_margins(split): + """Explicit dimensions and individual margin edges win over defaults.""" + margin = {"l": 17, "t": 0} + options = dict(title="Event", width=950, height=620, autosize=False, margin=margin) + layout = dual_figure3d([], [], **options).layout if split else layout3d(**options) + assert (layout.width, layout.height, layout.autosize) == (950, 620, False) + assert layout.margin.l == 17 + assert layout.margin.t == 0 + assert margin == {"l": 17, "t": 0} + + +def test_dual_layout_preserves_input_and_allows_explicit_overrides(): + """Split figures copy their base layout and distinguish omitted sizes.""" + layout = layout3d(width=950, height=620, margin={"t": 12, "l": 17}) + before = deepcopy(layout.to_plotly_json()) + preserved = dual_figure3d([], [], layout=layout, titles=["Reco", "Truth"]) + cleared = dual_figure3d( + [], [], layout=layout, width=None, height=None, margin={"r": 8} + ) + assert (preserved.layout.width, preserved.layout.height) == (950, 620) + assert preserved.layout.margin.to_plotly_json() == before["margin"] + assert cleared.layout.width is None + assert cleared.layout.height is None + assert cleared.layout.margin.l == 17 + assert cleared.layout.margin.r == 8 + assert layout.to_plotly_json() == before + + +def test_dual_layout_reserves_space_for_both_title_levels(): + """Figure titles and panel titles share the top margin without mutation.""" + panels = dual_figure3d([], [], titles=["Reco", "Truth"]) + both = dual_figure3d( + [], [], titles=["Reco", "Truth"], title={"text": "Event"}, margin={"l": 5} + ) + assert panels.layout.margin.t == 42 + assert both.layout.margin.t == 102 + assert both.layout.margin.l == 5 + assert both.layout.title.text == "Event" + assert [annotation.text for annotation in both.layout.annotations] == [ + "Reco", + "Truth", + ] + assert both.layout.width is None + assert both.layout.height == 800 + assert both.layout.autosize is True + + def test_layout3d_uses_point_ranges_meta_and_dark_options(monkeypatch): point_layout = layout3d(ranges=POINTS, titles=["a", "b", "c"], dark=True) meta_layout = layout3d(meta=MetaStub()) diff --git a/test/test_vis/test_scene/test_scene.py b/test/test_vis/test_scene/test_scene.py index 615aab9a..97dab5cb 100644 --- a/test/test_vis/test_scene/test_scene.py +++ b/test/test_vis/test_scene/test_scene.py @@ -447,6 +447,12 @@ def test_plotly_backend_validates_and_renders_views(): ) dual_figure = backend.render(dual_scene, synchronize=False) assert len(dual_figure.data) == 2 + assert dual_figure.layout.margin.t == 42 + titled = backend.render(dual_scene, title="Event", margin={"l": 9}) + assert titled.layout.margin.t == 102 + assert titled.layout.margin.l == 9 + assert titled.layout.width is None + assert titled.layout.height == 800 invalid_scene = Scene([SceneView(str(i)) for i in range(3)]) with pytest.raises(ValueError, match="at most two"):