Make sure that strings get converted to DimRole for comparison to fix…

nbs_viewer/models/plot/spec/projection.py

@@ -95,6 +95,12 @@ def __post_init__(self) -> None:
             )
         if len(self.roles) != self.ndim:
             raise ValueError("roles length must match ndim")
+        # A bare "sum" compares equal to DimRole.SUM but is not it, and the
+        # reduce stage matches roles with ``is``. Every projection passes
+        # through here, so this is where roles become DimRole.
+        object.__setattr__(
+            self, "roles", tuple(DimRole(role) for role in self.roles)
+        )
         if len(self.indices) != self.ndim:
             raise ValueError("indices length must match ndim")
         if not self.axis_order:

nbs_viewer/models/plot/view_intent.py

@@ -49,7 +49,9 @@ def _validated_reduce(
             raise ValueError(
                 f"reduce roles must be INDEX/SUM/MEAN, got {role}"
             )
-    return roles, indices
+    # The dimension rows edit this policy, and a QComboBox hands back the bare
+    # string rather than the DimRole member.
+    return tuple(DimRole(role) for role in roles), indices
 
 
 def _validated_plot_ndim(plot_ndim: int) -> int:

nbs_viewer/views/plot/mplCanvas/renderers.py

@@ -98,6 +98,7 @@ def create(
         cbar.set_label(label)
         colorbar_state["colorbar"] = cbar
         ImageRenderer.set_labels(axes, bundle)
+        ImageRenderer._set_limits(axes, bundle)
         print_debug(
             "ImageRenderer.create",
             f"shape={bundle.y.shape} {ttime.time() - t0:.4f}s",
@@ -116,11 +117,8 @@ def update(
         artist.set_data(bundle.y)
         if bundle.extent is not None:
             artist.set_extent(bundle.extent)
-            axes = artist.axes
-            if axes is not None:
-                left, right, bottom, top = bundle.extent
-                axes.set_xlim(left, right)
-                axes.set_ylim(bottom, top)
+            if artist.axes is not None:
+                ImageRenderer._set_limits(artist.axes, bundle)
         if autoscale:
             finite = bundle.y[np.isfinite(bundle.y)]
             if finite.size > 0:
@@ -134,6 +132,17 @@ def update(
             category="plots",
         )
 
+    @staticmethod
+    def _set_limits(axes: Axes, bundle: PlotBundle) -> None:
+        # Both paths, as for a mesh. ``imshow`` scales the axes itself, but
+        # only while they still autoscale, and the first ``set_xlim`` here
+        # ends that -- after which a newly created image would keep the
+        # previous one's limits.
+        if bundle.extent is not None:
+            left, right, bottom, top = bundle.extent
+            axes.set_xlim(left, right)
+            axes.set_ylim(bottom, top)
+
     @staticmethod
     def set_labels(axes: Axes, bundle: PlotBundle) -> None:
         frame = bundle.view_frame()

tests/test_view_spec.py

@@ -479,3 +479,51 @@ def test_construction_fixes_duplicate_plot_x():
         indices=(0, 0),
     )
     assert sum(1 for r in spec.roles if r == DimRole.PLOT_X) == 1
+
+
+def test_a_role_given_as_a_plain_string_becomes_a_DimRole():
+    """
+    Bug: a reduce axis set from the UI never reduced.
+
+    ``DimRole`` subclasses ``str``, and ``QComboBox.currentData`` round-trips
+    its items through a ``QVariant``, which hands back a plain ``str``. That
+    value passes every ``==`` and ``in`` check on the way down -- validation,
+    role resolution, the load slice -- and fails only at ``reduce_to_plane``,
+    which matches roles with ``is``. The axis was then neither indexed away
+    nor collapsed, and the pack rejected the 3-D result.
+
+    Normalising here rather than loosening that comparison is what makes
+    "roles on a Projection are ``DimRole``" an invariant instead of a habit:
+    every projection is built or ``replace``d through this constructor.
+    """
+    view = Projection(
+        ndim=3,
+        plot_ndim=2,
+        roles=("sum", "plot_y", "plot_x"),
+        indices=(0, 0, 0),
+    )
+
+    assert view.roles == (DimRole.SUM, DimRole.PLOT_Y, DimRole.PLOT_X)
+    assert all(role is DimRole(role) for role in view.roles)
+
+
+def test_with_slice_role_normalises_a_plain_string():
+    """
+    The edit the dimension rows make is the one that carried the bare string.
+    """
+    view = ViewIntent(plot_ndim=2).project(3)
+
+    edited = view.with_slice_role(0, "mean")
+
+    assert edited.roles[0] is DimRole.MEAN
+
+
+def test_view_intent_keeps_reduce_roles_as_DimRole():
+    """
+    The intent holds the policy the rows edited, so it normalises it too.
+    """
+    intent = ViewIntent(plot_ndim=2)
+
+    assert intent.set_reduce(("sum",), (3,))
+    assert intent.reduce_roles == (DimRole.SUM,)
+    assert intent.reduce_roles[0] is DimRole.SUM

tests/test_widgets.py

@@ -163,3 +163,121 @@ def test_the_roi_window_opens_and_offers_profile_axes(
     ]
     assert axes, "a rank-3 key offered no profile axis"
     assert set(axes) <= {0, 1, 2}
+
+
+@pytest.mark.parametrize(
+    "role_name, reduce_name", [("SUM", "sum"), ("MEAN", "mean")]
+)
+def test_the_reduce_combo_actually_reduces_the_slider_axis(
+    plot_widgets, role_name, reduce_name
+):
+    """
+    Bug: picking Sum or Mean on a slider axis produced no plot at all.
+
+    ``DimRole`` subclasses ``str`` and the combo stores its roles as item
+    data, which Qt round-trips through a ``QVariant`` and hands back as a
+    plain ``str``. Every check between the combo and the reduce stage
+    compares with ``==`` or ``in`` and so accepted it; the reduce stage
+    matches with ``is`` and so skipped the axis, leaving a rank-3 array for
+    a pack that only takes planes -- "Unsupported plot dimensionality: 3".
+
+    Only a widget can reach this: the model layer is handed a real
+    ``DimRole`` by every caller that is not a ``QComboBox``.
+    """
+    import numpy as np
+
+    from nbs_viewer.models.plot.plane.roles import DimRole
+    from nbs_viewer.views.plot.controls.dimension import DimensionControl
+
+    presenter, session, canvas = plot_widgets
+    session.selection.set_selected_keys(["en_energy"], ["detector_cube"], [])
+    session.view_intent.set_plot_ndim(2)
+    dimension_control = DimensionControl(presenter, canvas)
+    _pump()
+    dimension_control.create_sliders()
+
+    assert dimension_control._slice_rows, "no slider axis to reduce"
+    row = dimension_control._slice_rows[0]
+    reduced_axis = row.storage_axis
+    trace = next(iter(session.traces.values()))
+    block = trace.run.load(
+        "detector_cube", dims=trace.request.dims, xkeys=("en_energy",)
+    )
+
+    role = getattr(DimRole, role_name)
+    row.role_combo.setCurrentIndex(row.role_combo.findData(role))
+    _pump()
+
+    assert trace.request.view.roles[reduced_axis] is role
+    bundle = trace.fetch()
+    assert bundle.ndim == 2
+    expected = getattr(block, reduce_name)(
+        dim=trace.request.dims[reduced_axis]
+    )
+    np.testing.assert_allclose(
+        np.sort(bundle.y, axis=None), np.sort(expected.values, axis=None)
+    )
+
+
+def test_a_re_created_image_rescales_the_axes(qapp):
+    """
+    Bug: swapping the plot axes left the image drawn against stale limits.
+
+    ``imshow`` sets the axis limits only while the axes still autoscale, and
+    ``set_xlim`` / ``set_ylim`` turn that off for good. The update path calls
+    both, so the *next* image created on those axes -- which is what an
+    orientation change produces, since it tears the 2-D axes down first --
+    inherited the limits of the orientation before it, and the plane was
+    drawn cut off until the next slider step took the update path again.
+    """
+    import numpy as np
+    from matplotlib.figure import Figure
+
+    from nbs_viewer.models.plot.spec.bundle import PlotBundle
+    from nbs_viewer.views.plot.mplCanvas.renderers import ImageRenderer
+
+    tall = PlotBundle.from_2d(
+        np.zeros((32, 4)), [np.arange(32.0), np.arange(4.0)], ["row", "col"]
+    )
+    wide = PlotBundle.from_2d(
+        np.zeros((4, 32)), [np.arange(4.0), np.arange(32.0)], ["col", "row"]
+    )
+    fig = Figure()
+    axes = fig.add_subplot(111)
+    state = {}
+
+    artist, _cbar = ImageRenderer.create(axes, fig, tall, "y", state)
+    ImageRenderer.update(artist, tall, True, state)
+    artist.remove()
+    ImageRenderer.create(axes, fig, wide, "y", state)
+
+    left, right, bottom, top = wide.extent
+    assert axes.get_xlim() == (left, right)
+    assert axes.get_ylim() == (bottom, top)
+
+
+def test_swapping_the_plot_axes_rescales_the_drawn_plane(plot_widgets):
+    """
+    The gesture the stale limits were reported from, end to end.
+
+    Stepping the index slider first is what makes this fail: it renders
+    through the update path, which is what turns the axes' autoscaling off.
+    """
+    from nbs_viewer.views.plot.controls.dimension import DimensionControl
+
+    presenter, session, canvas = plot_widgets
+    session.selection.set_selected_keys(["en_energy"], ["detector_cube"], [])
+    session.view_intent.set_plot_ndim(2)
+    dimension_control = DimensionControl(presenter, canvas)
+    _pump()
+    dimension_control.create_sliders()
+
+    dimension_control._slice_rows[0].slider.setValue(1)
+    _pump()
+    dimension_control._plot_rows[0].move_down_requested.emit()
+    _pump()
+
+    trace = next(iter(session.traces.values()))
+    left, right, bottom, top = trace.last_bundle.extent
+    assert canvas.axes.get_xlim() == (left, right)
+    assert canvas.axes.get_ylim() == (bottom, top)