diff --git a/nbs_viewer/models/plot/spec/projection.py b/nbs_viewer/models/plot/spec/projection.py index 8764b85..7ff6e0a 100644 --- a/nbs_viewer/models/plot/spec/projection.py +++ b/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: diff --git a/nbs_viewer/models/plot/view_intent.py b/nbs_viewer/models/plot/view_intent.py index 5f5a4bb..70d79cc 100644 --- a/nbs_viewer/models/plot/view_intent.py +++ b/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: diff --git a/nbs_viewer/views/plot/mplCanvas/renderers.py b/nbs_viewer/views/plot/mplCanvas/renderers.py index 764e999..49d9eb9 100644 --- a/nbs_viewer/views/plot/mplCanvas/renderers.py +++ b/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() diff --git a/pixi.lock b/pixi.lock index ad1d1b9..ae2b962 100644 --- a/pixi.lock +++ b/pixi.lock @@ -5601,8 +5601,8 @@ packages: timestamp: 1757782706200 - pypi: ./ name: nbs-viewer - version: 0.3.5.dev189+gb1e33b802.d20260916 - sha256: 714decd9f8e8009e007a9951f3fbe01c5160567bb0d71e223439e370f9942757 + version: 0.3.5.dev221+g4eb713de6 + sha256: 0d100993c98b868cd973e51f54c6c73c8314f433a002fa16e85ec210effc0ca4 requires_dist: - asteval - bluesky-kafka diff --git a/tests/test_view_spec.py b/tests/test_view_spec.py index 3ffe290..d07d186 100644 --- a/tests/test_view_spec.py +++ b/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 diff --git a/tests/test_widgets.py b/tests/test_widgets.py index c78c012..a1a2c0b 100644 --- a/tests/test_widgets.py +++ b/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)