From 0fbe95af9d39e30bd6cd6d7bc77c5fb704e7c95b Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Sat, 11 Jan 2025 21:45:14 -0600 Subject: [PATCH 01/33] Convert LUTView to Model-View --- examples/notebook.ipynb | 49 ++------ src/ndv/_views/_jupyter/_array_view.py | 30 +++-- src/ndv/_views/_qt/_array_view.py | 24 +++- src/ndv/_views/_vispy/_histogram.py | 6 +- src/ndv/_views/_wx/_array_view.py | 12 +- src/ndv/_views/bases/_lut_view.py | 69 ++++++----- .../_views/bases/graphics/_canvas_elements.py | 28 ++++- src/ndv/controllers/_array_viewer.py | 10 +- src/ndv/controllers/_channel_controller.py | 116 ++---------------- 9 files changed, 139 insertions(+), 205 deletions(-) diff --git a/examples/notebook.ipynb b/examples/notebook.ipynb index f0f1b705..c14bf3bb 100644 --- a/examples/notebook.ipynb +++ b/examples/notebook.ipynb @@ -2,63 +2,34 @@ "cells": [ { "cell_type": "code", - "execution_count": 5, + "execution_count": null, "metadata": {}, - "outputs": [ - { - "data": { - "application/vnd.jupyter.widget-view+json": { - "model_id": "f2fe7ce4f36847ffaba8c042bf85b76d", - "version_major": 2, - "version_minor": 0 - }, - "text/plain": [ - "RFBOutputContext()" - ] - }, - "metadata": {}, - "output_type": "display_data" - }, - { - "data": { - "application/vnd.jupyter.widget-view+json": { - "model_id": "92985e7df82f47cabfe3ab5ce9ab0498", - "version_major": 2, - "version_minor": 0 - }, - "text/plain": [ - "VBox(children=(Label(value='.ndarray (60, 2, 256, 256), uint16, 15.73MB'), CanvasBackend(css_height='600px', c…" - ] - }, - "metadata": {}, - "output_type": "display_data" - } - ], + "outputs": [], "source": [ "from ndv import data, imshow\n", "\n", "viewer = imshow(data.cells3d())\n", - "viewer.model.channel_mode = \"composite\"\n", - "viewer.model.current_index.update({0: 32})" + "viewer.display_model.channel_mode = \"composite\"\n", + "viewer.display_model.current_index.update({0: 32})" ] }, { "cell_type": "code", - "execution_count": 2, + "execution_count": null, "metadata": {}, "outputs": [], "source": [ - "viewer.model.visible_axes = (0, 3)" + "viewer.display_model.visible_axes = (0, 3)" ] }, { "cell_type": "code", - "execution_count": 3, + "execution_count": null, "metadata": {}, "outputs": [], "source": [ - "viewer.model.default_lut.cmap = \"cubehelix\"\n", - "viewer.model.channel_mode = \"grayscale\"" + "viewer.display_model.default_lut.cmap = \"cubehelix\"\n", + "viewer.display_model.channel_mode = \"grayscale\"" ] } ], @@ -78,7 +49,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.12.7" + "version": "3.12.0" } }, "nbformat": 4, diff --git a/src/ndv/_views/_jupyter/_array_view.py b/src/ndv/_views/_jupyter/_array_view.py index 6888c619..4b26f078 100644 --- a/src/ndv/_views/_jupyter/_array_view.py +++ b/src/ndv/_views/_jupyter/_array_view.py @@ -3,7 +3,6 @@ import warnings from typing import TYPE_CHECKING, Any, cast -import cmap import ipywidgets as widgets from ndv._views.bases import ArrayView, LutView @@ -12,6 +11,7 @@ if TYPE_CHECKING: from collections.abc import Container, Hashable, Mapping, Sequence + import cmap from vispy.app.backends import _jupyter_rfb from ndv._types import AxisKey @@ -59,16 +59,20 @@ def __init__(self) -> None: # ------------------ emit changes to the controller ------------------ def _on_clims_changed(self, change: dict[str, Any]) -> None: - self.climsChanged.emit(self._clims.value) + if self._model: + self._model.clims = self._clims.value def _on_visible_changed(self, change: dict[str, Any]) -> None: - self.visibilityChanged.emit(self._visible.value) + if self._model: + self._model.visible = self._visible.value def _on_cmap_changed(self, change: dict[str, Any]) -> None: - self.cmapChanged.emit(cmap.Colormap(self._cmap.value)) + if self._model: + self._model.cmap = self._cmap.value def _on_autoscale_changed(self, change: dict[str, Any]) -> None: - self.autoscaleChanged.emit(self._auto_clim.value) + if self._model: + self._model.autoscale = self._auto_clim.value # ------------------ receive changes from the controller --------------- @@ -79,20 +83,20 @@ def set_channel_name(self, name: str) -> None: # to avoid loops, unnecessary updates, and unexpected behavior def set_auto_scale(self, auto: bool) -> None: - with self.autoscaleChanged.blocked(): - self._auto_clim.value = auto + # with self.autoscaleChanged.blocked(): + self._auto_clim.value = auto def set_colormap(self, cmap: cmap.Colormap) -> None: - with self.cmapChanged.blocked(): - self._cmap.value = cmap.name.split(":")[-1] # FIXME: this is a hack + # with self.cmapChanged.blocked(): + self._cmap.value = cmap.name.split(":")[-1] # FIXME: this is a hack def set_clims(self, clims: tuple[float, float]) -> None: - with self.climsChanged.blocked(): - self._clims.value = clims + # with self.climsChanged.blocked(): + self._clims.value = clims def set_channel_visible(self, visible: bool) -> None: - with self.visibilityChanged.blocked(): - self._visible.value = visible + # with self.visibilityChanged.blocked(): + self._visible.value = visible def set_gamma(self, gamma: float) -> None: pass diff --git a/src/ndv/_views/_qt/_array_view.py b/src/ndv/_views/_qt/_array_view.py index 518efd22..3e8f880b 100644 --- a/src/ndv/_views/_qt/_array_view.py +++ b/src/ndv/_views/_qt/_array_view.py @@ -126,10 +126,10 @@ def __init__(self) -> None: super().__init__() self._qwidget = _QLUTWidget() # TODO: use emit_fast - self._qwidget.visible.toggled.connect(self.visibilityChanged.emit) - self._qwidget.cmap.currentColormapChanged.connect(self.cmapChanged.emit) - self._qwidget.clims.valueChanged.connect(self.climsChanged.emit) - self._qwidget.auto_clim.toggled.connect(self.autoscaleChanged.emit) + self._qwidget.visible.toggled.connect(self._on_q_visibility_changed) + self._qwidget.cmap.currentColormapChanged.connect(self._on_q_cmap_changed) + self._qwidget.clims.valueChanged.connect(self._on_q_clims_changed) + self._qwidget.auto_clim.toggled.connect(self._on_q_auto_changed) def frontend_widget(self) -> QWidget: return self._qwidget @@ -158,6 +158,22 @@ def set_visible(self, visible: bool) -> None: def close(self) -> None: self._qwidget.close() + def _on_q_visibility_changed(self, visible: bool) -> None: + if self._model: + self._model.visible = visible + + def _on_q_cmap_changed(self, cmap: cmap.Colormap) -> None: + if self._model: + self._model.cmap = cmap + + def _on_q_clims_changed(self, clims: tuple[float, float]) -> None: + if self._model: + self._model.clims = clims + + def _on_q_auto_changed(self, autoscale: bool) -> None: + if self._model: + self._model.autoscale = autoscale + class _QDimsSliders(QWidget): currentIndexChanged = Signal() diff --git a/src/ndv/_views/_vispy/_histogram.py b/src/ndv/_views/_vispy/_histogram.py index 38437eae..a0227fcc 100644 --- a/src/ndv/_views/_vispy/_histogram.py +++ b/src/ndv/_views/_vispy/_histogram.py @@ -317,7 +317,8 @@ def on_mouse_move(self, event: MouseMoveEvent) -> bool: newlims = (min(self._clims[1], c), self._clims[1]) elif self._grabbed is Grabbable.RIGHT_CLIM: newlims = (self._clims[0], max(self._clims[0], c)) - self.climsChanged.emit(newlims) + if self.model: + self.model.clims = newlims return False if self._grabbed is Grabbable.GAMMA: @@ -329,7 +330,8 @@ def on_mouse_move(self, event: MouseMoveEvent) -> bool: y = self._to_plot_coords(pos)[0 if self._vertical else 1] if y < np.maximum(y0, 0) or y > y1: return False - self.gammaChanged.emit(-np.log2(y / y1)) + if self.model: + self.model.gamma = -np.log2(y / y1) return False self.get_cursor(pos).apply_to(self) diff --git a/src/ndv/_views/_wx/_array_view.py b/src/ndv/_views/_wx/_array_view.py index b8ee969f..d621a590 100644 --- a/src/ndv/_views/_wx/_array_view.py +++ b/src/ndv/_views/_wx/_array_view.py @@ -65,16 +65,20 @@ def __init__(self, parent: wx.Window) -> None: # Event Handlers def _on_visible_changed(self, event: wx.CommandEvent) -> None: - self.visibilityChanged.emit(self._wxwidget.visible.GetValue()) + if self._model: + self._model.visible = self._wxwidget.visible.GetValue() def _on_cmap_changed(self, event: wx.CommandEvent) -> None: - self.cmapChanged.emit(self._wxwidget.cmap.GetValue()) + if self._model: + self._model.cmap = self._wxwidget.cmap.GetValue() def _on_clims_changed(self, event: wx.CommandEvent) -> None: - self.climsChanged.emit(self._wxwidget.clims.GetValues()) + if self._model: + self._model.clims = self._wxwidget.clims.GetValues() def _on_autoscale_changed(self, event: wx.CommandEvent) -> None: - self.autoscaleChanged.emit(self._wxwidget.auto_clim.GetValue()) + if self._model: + self._model.autoscale = self._wxwidget.auto_clim.GetValue() # Public Methods def frontend_widget(self) -> wx.Window: diff --git a/src/ndv/_views/bases/_lut_view.py b/src/ndv/_views/bases/_lut_view.py index d621c92b..3b5b8020 100644 --- a/src/ndv/_views/bases/_lut_view.py +++ b/src/ndv/_views/bases/_lut_view.py @@ -1,8 +1,8 @@ from abc import abstractmethod -from typing import final import cmap -from psygnal import Signal + +from ndv.models._lut_model import LUTModel from ._view_base import Viewable @@ -10,11 +10,10 @@ class LutView(Viewable): """Manages LUT properties (contrast, colormap, etc...) in a view object.""" - visibilityChanged = Signal(bool) - autoscaleChanged = Signal(bool) - cmapChanged = Signal(cmap.Colormap) - climsChanged = Signal(tuple) - gammaChanged = Signal(float) + _model: LUTModel | None = None + + def __init__(self, model: LUTModel | None = None) -> None: + self.model = model @abstractmethod def set_channel_name(self, name: str) -> None: @@ -49,29 +48,33 @@ def set_gamma(self, gamma: float) -> None: """Set the gamma value of the LUT.""" return None - # These methods apply a value to the view without re-emitting the signal. - - @final - def set_auto_scale_without_signal(self, auto: bool) -> None: - with self.autoscaleChanged.blocked(): - self.set_auto_scale(auto) - - @final - def set_colormap_without_signal(self, cmap: cmap.Colormap) -> None: - with self.cmapChanged.blocked(): - self.set_colormap(cmap) - - @final - def set_clims_without_signal(self, clims: tuple[float, float]) -> None: - with self.climsChanged.blocked(): - self.set_clims(clims) - - @final - def set_gamma_without_signal(self, gamma: float) -> None: - with self.gammaChanged.blocked(): - self.set_gamma(gamma) - - @final - def set_channel_visible_without_signal(self, visible: bool) -> None: - with self.visibilityChanged.blocked(): - self.set_channel_visible(visible) + @property + def model(self) -> LUTModel | None: + return self._model + + @model.setter + def model(self, model: LUTModel | None) -> None: + if self._model is not None: + self._model.events.autoscale.disconnect(self.set_auto_scale) + self._model.events.clims.disconnect(self.set_clims) + self._model.events.cmap.disconnect(self.set_colormap) + self._model.events.gamma.disconnect(self.set_gamma) + self._model.events.visible.disconnect(self.set_channel_visible) + self._model = model + if self._model is not None: + self._model.events.autoscale.connect(self.set_auto_scale) + self._model.events.clims.connect(self.set_clims) + self._model.events.cmap.connect(self.set_colormap) + self._model.events.gamma.connect(self.set_gamma) + self._model.events.visible.connect(self.set_channel_visible) + + self.synchronize() + + def synchronize(self) -> None: + if model := self._model: + self.set_auto_scale(bool(model.autoscale)) + if model.clims: + self.set_clims(model.clims) + self.set_colormap(model.cmap) + self.set_gamma(model.gamma) + self.set_channel_visible(model.visible) diff --git a/src/ndv/_views/bases/graphics/_canvas_elements.py b/src/ndv/_views/bases/graphics/_canvas_elements.py index 4e7ced55..80a84b84 100644 --- a/src/ndv/_views/bases/graphics/_canvas_elements.py +++ b/src/ndv/_views/bases/graphics/_canvas_elements.py @@ -3,6 +3,8 @@ from abc import abstractmethod from typing import TYPE_CHECKING, Any +from ndv._views.bases._lut_view import LutView + from ._mouseable import Mouseable if TYPE_CHECKING: @@ -60,7 +62,7 @@ def remove(self) -> None: """Removes the element from the canvas.""" -class ImageHandle(CanvasElement): +class ImageHandle(CanvasElement, LutView): @abstractmethod def data(self) -> np.ndarray: ... @abstractmethod @@ -78,6 +80,30 @@ def cmap(self) -> _cmap.Colormap: ... @abstractmethod def set_cmap(self, cmap: _cmap.Colormap) -> None: ... + # -- LutView methods -- # + def close(self) -> None: + pass + + def frontend_widget(self) -> Any: + return None + + def set_channel_name(self, name: str) -> None: + pass + + def set_auto_scale(self, checked: bool) -> None: + # TODO: Make this computation alter the slider... + if checked: + d = self.data() + self.set_clims((d.min(), d.max())) + + def set_colormap(self, cmap: _cmap.Colormap) -> None: + self.set_cmap(cmap) + + def set_channel_visible(self, visible: bool) -> None: + self.set_visible(visible) + + # set_clims, set_gamma reused above + class RoiHandle(CanvasElement): @abstractmethod diff --git a/src/ndv/controllers/_array_viewer.py b/src/ndv/controllers/_array_viewer.py index bd92430c..e84af016 100644 --- a/src/ndv/controllers/_array_viewer.py +++ b/src/ndv/controllers/_array_viewer.py @@ -165,10 +165,10 @@ def _add_histogram(self) -> None: histogram_cls = _app.get_histogram_canvas_class() # will raise if not supported self._histogram = histogram_cls() self._view.add_histogram(self._histogram.frontend_widget()) - for view in self._lut_controllers.values(): - view.add_lut_view(self._histogram) + for ctrl in self._lut_controllers.values(): + ctrl.add_lut_view(self._histogram) # FIXME: hack - if handles := view.handles: + if handles := ctrl.handles: data = handles[0].data() counts, edges = _calc_hist_bins(data) self._histogram.set_data(counts, edges) @@ -232,7 +232,7 @@ def _fully_synchronize_view(self) -> None: self._clear_canvas() self._update_canvas() for lut_ctr in self._lut_controllers.values(): - lut_ctr._update_view_from_model() + lut_ctr.synchronize() self._update_hist_domain_for_dtype() def _on_model_visible_axes_changed(self) -> None: @@ -335,7 +335,7 @@ def _update_canvas(self) -> None: lut_views.append(self._histogram) self._lut_controllers[key] = lut_ctrl = ChannelController( key=key, - model=model, + lut_model=model, views=lut_views, ) diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index 75ed3e2b..73d918fc 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -6,7 +6,6 @@ if TYPE_CHECKING: from collections.abc import Iterable, Sequence - import cmap import numpy as np from ndv._views.bases import LutView @@ -25,121 +24,30 @@ class ChannelController: that displays the data, all for a single "channel" extracted from the data. """ - def __init__(self, key: LutKey, model: LUTModel, views: Sequence[LutView]) -> None: + def __init__( + self, key: LutKey, lut_model: LUTModel, views: Sequence[LutView] + ) -> None: self.key = key self.lut_views: list[LutView] = [] - self.lut_model = model + self.lut_model = lut_model self.handles: list[ImageHandle] = [] - for v in views: self.add_lut_view(v) - # connect model changes to view callbacks that update the view - self.lut_model.events.cmap.connect(self._on_model_cmap_changed) - self.lut_model.events.clims.connect(self._on_model_clims_changed) - self.lut_model.events.autoscale.connect(self._on_model_autoscale_changed) - self.lut_model.events.visible.connect(self._on_model_visible_changed) - self.lut_model.events.gamma.connect(self._on_model_gamma_changed) - def add_lut_view(self, view: LutView) -> None: """Add a LUT view to the controller.""" + view.model = self.lut_model self.lut_views.append(view) - # connect view changes to controller callbacks that update the model - view.visibilityChanged.connect(self._on_view_lut_visible_changed) - view.autoscaleChanged.connect(self._on_view_lut_autoscale_changed) - view.cmapChanged.connect(self._on_view_lut_cmap_changed) - view.climsChanged.connect(self._on_view_lut_clims_changed) - view.gammaChanged.connect(self._on_view_lut_gamma_changed) - self._update_view_from_model(view) - - def _on_model_clims_changed(self, clims: tuple[float, float]) -> None: - """The contrast limits in the model have changed.""" - for v in self.lut_views: - v.set_clims_without_signal(clims) - for handle in self.handles: - handle.set_clims(clims) - - def _on_model_gamma_changed(self, gamma: float) -> None: - """The gamma value in the model has changed.""" - for view in self.lut_views: - view.set_gamma_without_signal(gamma) - for handle in self.handles: - handle.set_gamma(gamma) + self.synchronize(view) - def _on_model_autoscale_changed(self, autoscale: bool) -> None: - """The autoscale setting in the model has changed.""" - for view in self.lut_views: - view.set_auto_scale_without_signal(autoscale) - if autoscale: - for handle in self.handles: - d = handle.data() - handle.set_clims((d.min(), d.max())) - - def _on_model_cmap_changed(self, cmap: cmap.Colormap) -> None: - """The colormap in the model has changed.""" - for view in self.lut_views: - view.set_colormap_without_signal(cmap) - for handle in self.handles: - handle.set_cmap(cmap) - - def _on_model_visible_changed(self, visible: bool) -> None: - """The visibility in the model has changed.""" - for view in self.lut_views: - view.set_channel_visible_without_signal(visible) - for handle in self.handles: - handle.set_visible(visible) - - def _update_view_from_model(self, *views: LutView) -> None: + def synchronize(self, *views: LutView) -> None: """Make sure the view matches the model.""" _views: Iterable[LutView] = views or self.lut_views + name = str(self.key) if self.key is not None else "" for view in _views: - view.set_colormap_without_signal(self.lut_model.cmap) - if self.lut_model.clims: - view.set_clims_without_signal(self.lut_model.clims) - # TODO: handle more complex autoscale types - view.set_auto_scale_without_signal(bool(self.lut_model.autoscale)) - view.set_channel_visible_without_signal(True) - name = str(self.key) if self.key is not None else "" + view.synchronize() view.set_channel_name(name) - def _on_view_lut_visible_changed(self, visible: bool, key: LutKey = None) -> None: - """The visibility checkbox in the LUT widget has changed.""" - for handle in self.handles: - handle.set_visible(visible) - - def _on_view_lut_autoscale_changed( - self, autoscale: bool, key: LutKey = None - ) -> None: - """The autoscale checkbox in the LUT widget has changed.""" - self.lut_model.autoscale = autoscale - for view in self.lut_views: - view.set_auto_scale_without_signal(autoscale) - - if autoscale: - # TODO: or should we have a global min/max across all handles for this key? - for handle in self.handles: - data = handle.data() - # update the model with the new clim values - self.lut_model.clims = (data.min(), data.max()) - - def _on_view_lut_cmap_changed( - self, cmap: cmap.Colormap, key: LutKey = None - ) -> None: - """The colormap in the LUT widget has changed.""" - for handle in self.handles: - handle.set_cmap(cmap) # actually apply it to the Image texture - self.lut_model.cmap = cmap # update the model as well - - def _on_view_lut_clims_changed(self, clims: tuple[float, float]) -> None: - """The contrast limits slider in the LUT widget has changed.""" - self.lut_model.clims = clims - # when the clims are manually adjusted in the view, we turn off autoscale - self.lut_model.autoscale = False - - def _on_view_lut_gamma_changed(self, gamma: float) -> None: - """The gamma slider in the LUT widget has changed.""" - self.lut_model.gamma = gamma - def update_texture_data(self, data: np.ndarray) -> None: """Update the data in the image handle.""" # WIP: @@ -160,13 +68,13 @@ def update_texture_data(self, data: np.ndarray) -> None: def add_handle(self, handle: ImageHandle) -> None: """Add an image texture handle to the controller.""" + handle.model = self.lut_model self.handles.append(handle) - handle.set_cmap(self.lut_model.cmap) + self.add_lut_view(handle) + if self.lut_model.autoscale: data = handle.data() self.lut_model.clims = (data.min(), data.max()) - if self.lut_model.clims: - handle.set_clims(self.lut_model.clims) def get_value_at_index(self, idx: tuple[int, ...]) -> float | None: """Get the value of the data at the given index.""" From cc931db9755b9235c734731ddd3adbb26c66998c Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Sun, 12 Jan 2025 10:53:36 -0500 Subject: [PATCH 02/33] rename views --- src/ndv/__init__.py | 2 +- src/ndv/_types.py | 2 +- src/ndv/controllers/_array_viewer.py | 4 ++-- src/ndv/controllers/_channel_controller.py | 4 ++-- src/ndv/util.py | 2 +- src/ndv/v1/_old_viewer.py | 6 +++--- src/ndv/v1/_qt/_lut_control.py | 2 +- src/ndv/{_views => views}/__init__.py | 0 src/ndv/{_views => views}/_app.py | 10 +++++----- src/ndv/{_views => views}/_jupyter/__init__.py | 0 src/ndv/{_views => views}/_jupyter/_array_view.py | 2 +- src/ndv/{_views => views}/_pygfx/__init__.py | 0 src/ndv/{_views => views}/_pygfx/_array_canvas.py | 6 +++--- src/ndv/{_views => views}/_qt/__init__.py | 0 src/ndv/{_views => views}/_qt/_array_view.py | 2 +- src/ndv/{_views => views}/_qt/_save_button.py | 0 src/ndv/{_views => views}/_qt/spin.gif | Bin src/ndv/{_views => views}/_vispy/__init__.py | 0 src/ndv/{_views => views}/_vispy/_array_canvas.py | 8 ++++---- src/ndv/{_views => views}/_vispy/_histogram.py | 4 ++-- src/ndv/{_views => views}/_vispy/_plot_widget.py | 0 src/ndv/{_views => views}/_vispy/_utils.py | 0 src/ndv/{_views => views}/_wx/__init__.py | 0 src/ndv/{_views => views}/_wx/_array_view.py | 6 +++--- src/ndv/{_views => views}/_wx/_labeled_slider.py | 0 src/ndv/{_views => views}/_wx/range_slider.py | 0 src/ndv/{_views => views}/bases/__init__.py | 0 src/ndv/{_views => views}/bases/_array_view.py | 4 ++-- src/ndv/{_views => views}/bases/_lut_view.py | 0 src/ndv/{_views => views}/bases/_view_base.py | 0 .../{_views => views}/bases/graphics/__init__.py | 0 src/ndv/{_views => views}/bases/graphics/_canvas.py | 6 +++--- .../bases/graphics/_canvas_elements.py | 2 +- .../{_views => views}/bases/graphics/_mouseable.py | 0 tests/conftest.py | 4 ++-- tests/test_controller.py | 10 +++++----- 36 files changed, 43 insertions(+), 43 deletions(-) rename src/ndv/{_views => views}/__init__.py (100%) rename src/ndv/{_views => views}/_app.py (98%) rename src/ndv/{_views => views}/_jupyter/__init__.py (100%) rename src/ndv/{_views => views}/_jupyter/_array_view.py (99%) rename src/ndv/{_views => views}/_pygfx/__init__.py (100%) rename src/ndv/{_views => views}/_pygfx/_array_canvas.py (99%) rename src/ndv/{_views => views}/_qt/__init__.py (100%) rename src/ndv/{_views => views}/_qt/_array_view.py (99%) rename src/ndv/{_views => views}/_qt/_save_button.py (100%) rename src/ndv/{_views => views}/_qt/spin.gif (100%) rename src/ndv/{_views => views}/_vispy/__init__.py (100%) rename src/ndv/{_views => views}/_vispy/_array_canvas.py (98%) rename src/ndv/{_views => views}/_vispy/_histogram.py (99%) rename src/ndv/{_views => views}/_vispy/_plot_widget.py (100%) rename src/ndv/{_views => views}/_vispy/_utils.py (100%) rename src/ndv/{_views => views}/_wx/__init__.py (100%) rename src/ndv/{_views => views}/_wx/_array_view.py (98%) rename src/ndv/{_views => views}/_wx/_labeled_slider.py (100%) rename src/ndv/{_views => views}/_wx/range_slider.py (100%) rename src/ndv/{_views => views}/bases/__init__.py (100%) rename src/ndv/{_views => views}/bases/_array_view.py (94%) rename src/ndv/{_views => views}/bases/_lut_view.py (100%) rename src/ndv/{_views => views}/bases/_view_base.py (100%) rename src/ndv/{_views => views}/bases/graphics/__init__.py (100%) rename src/ndv/{_views => views}/bases/graphics/_canvas.py (93%) rename src/ndv/{_views => views}/bases/graphics/_canvas_elements.py (98%) rename src/ndv/{_views => views}/bases/graphics/_mouseable.py (100%) diff --git a/src/ndv/__init__.py b/src/ndv/__init__.py index 5f309980..e9772b2a 100644 --- a/src/ndv/__init__.py +++ b/src/ndv/__init__.py @@ -8,9 +8,9 @@ __version__ = "uninstalled" from . import data -from ._views import run_app from .controllers import ArrayViewer from .models import DataWrapper from .util import imshow +from .views import run_app __all__ = ["ArrayViewer", "DataWrapper", "data", "imshow", "run_app"] diff --git a/src/ndv/_types.py b/src/ndv/_types.py index 27660487..9bfe5d86 100644 --- a/src/ndv/_types.py +++ b/src/ndv/_types.py @@ -14,7 +14,7 @@ from qtpy.QtCore import Qt from qtpy.QtWidgets import QWidget - from ndv._views.bases._view_base import Viewable + from ndv.views.bases._view_base import Viewable def _maybe_int(val: Any) -> Any: diff --git a/src/ndv/controllers/_array_viewer.py b/src/ndv/controllers/_array_viewer.py index e84af016..dc80bacc 100644 --- a/src/ndv/controllers/_array_viewer.py +++ b/src/ndv/controllers/_array_viewer.py @@ -5,12 +5,12 @@ import numpy as np -from ndv._views import _app from ndv.controllers._channel_controller import ChannelController from ndv.models._array_display_model import ArrayDisplayModel, ChannelMode from ndv.models._data_display_model import _ArrayDataDisplayModel from ndv.models._lut_model import LUTModel from ndv.models.data_wrappers import DataWrapper +from ndv.views import _app if TYPE_CHECKING: from typing import Any, Unpack @@ -18,8 +18,8 @@ from typing_extensions import TypeAlias from ndv._types import MouseMoveEvent - from ndv._views.bases import ArrayView, HistogramCanvas from ndv.models._array_display_model import ArrayDisplayModelKwargs + from ndv.views.bases import ArrayView, HistogramCanvas LutKey: TypeAlias = int | None diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index 73d918fc..72b8d014 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -8,9 +8,9 @@ import numpy as np - from ndv._views.bases import LutView - from ndv._views.bases.graphics._canvas_elements import ImageHandle from ndv.models._lut_model import LUTModel + from ndv.views.bases import LutView + from ndv.views.bases.graphics._canvas_elements import ImageHandle LutKey = int | None diff --git a/src/ndv/util.py b/src/ndv/util.py index f09617b4..84bd9aa2 100644 --- a/src/ndv/util.py +++ b/src/ndv/util.py @@ -4,8 +4,8 @@ from typing import TYPE_CHECKING, overload -from ndv._views._app import run_app from ndv.controllers import ArrayViewer +from ndv.views._app import run_app if TYPE_CHECKING: from typing import Any, Unpack diff --git a/src/ndv/v1/_old_viewer.py b/src/ndv/v1/_old_viewer.py index f0f9a4de..15ee3b34 100755 --- a/src/ndv/v1/_old_viewer.py +++ b/src/ndv/v1/_old_viewer.py @@ -12,7 +12,7 @@ from superqt import QCollapsible, QElidingLabel, QIconifyIcon, ensure_main_thread from superqt.utils import qthrottled, signals_blocked -from ndv._views import get_array_canvas_class +from ndv.views import get_array_canvas_class from ._old_data_wrapper import DataWrapper from ._qt._components import ( @@ -33,8 +33,8 @@ from qtpy.QtCore import QObject from qtpy.QtGui import QCloseEvent, QKeyEvent - from ndv._views.bases.graphics._canvas import ArrayCanvas - from ndv._views.bases.graphics._canvas_elements import ( + from ndv.views.bases.graphics._canvas import ArrayCanvas + from ndv.views.bases.graphics._canvas_elements import ( CanvasElement, ImageHandle, RoiHandle, diff --git a/src/ndv/v1/_qt/_lut_control.py b/src/ndv/v1/_qt/_lut_control.py index 37d439e5..24d14f6d 100644 --- a/src/ndv/v1/_qt/_lut_control.py +++ b/src/ndv/v1/_qt/_lut_control.py @@ -16,7 +16,7 @@ import cmap - from ndv._views.bases.graphics._canvas_elements import ImageHandle + from ndv.views.bases.graphics._canvas_elements import ImageHandle class CmapCombo(QColormapComboBox): diff --git a/src/ndv/_views/__init__.py b/src/ndv/views/__init__.py similarity index 100% rename from src/ndv/_views/__init__.py rename to src/ndv/views/__init__.py diff --git a/src/ndv/_views/_app.py b/src/ndv/views/_app.py similarity index 98% rename from src/ndv/_views/_app.py rename to src/ndv/views/_app.py index 6742bc9d..2406c3d6 100644 --- a/src/ndv/_views/_app.py +++ b/src/ndv/views/_app.py @@ -18,8 +18,8 @@ from IPython.core.interactiveshell import InteractiveShell - from ndv._views.bases import ArrayCanvas, ArrayView, HistogramCanvas - from ndv._views.bases.graphics._mouseable import Mouseable + from ndv.views.bases import ArrayCanvas, ArrayView, HistogramCanvas + from ndv.views.bases.graphics._mouseable import Mouseable GUI_ENV_VAR = "NDV_GUI_FRONTEND" @@ -329,7 +329,7 @@ def is_available() -> bool: def array_canvas_class() -> type[ArrayCanvas]: from vispy.app import use_app - from ndv._views._vispy._array_canvas import VispyArrayCanvas + from ndv.views._vispy._array_canvas import VispyArrayCanvas # these may not be necessary, since we likely have already called # create_app by this point and vispy will autodetect that. @@ -346,7 +346,7 @@ def array_canvas_class() -> type[ArrayCanvas]: @staticmethod def histogram_canvas_class() -> type[HistogramCanvas]: - from ndv._views._vispy._histogram import VispyHistogramCanvas + from ndv.views._vispy._histogram import VispyHistogramCanvas return VispyHistogramCanvas @@ -362,7 +362,7 @@ def is_available() -> bool: @staticmethod def array_canvas_class() -> type[ArrayCanvas]: - from ndv._views._pygfx._array_canvas import GfxArrayCanvas + from ndv.views._pygfx._array_canvas import GfxArrayCanvas return GfxArrayCanvas diff --git a/src/ndv/_views/_jupyter/__init__.py b/src/ndv/views/_jupyter/__init__.py similarity index 100% rename from src/ndv/_views/_jupyter/__init__.py rename to src/ndv/views/_jupyter/__init__.py diff --git a/src/ndv/_views/_jupyter/_array_view.py b/src/ndv/views/_jupyter/_array_view.py similarity index 99% rename from src/ndv/_views/_jupyter/_array_view.py rename to src/ndv/views/_jupyter/_array_view.py index 4b26f078..117ee2c7 100644 --- a/src/ndv/_views/_jupyter/_array_view.py +++ b/src/ndv/views/_jupyter/_array_view.py @@ -5,8 +5,8 @@ import ipywidgets as widgets -from ndv._views.bases import ArrayView, LutView from ndv.models._array_display_model import ChannelMode +from ndv.views.bases import ArrayView, LutView if TYPE_CHECKING: from collections.abc import Container, Hashable, Mapping, Sequence diff --git a/src/ndv/_views/_pygfx/__init__.py b/src/ndv/views/_pygfx/__init__.py similarity index 100% rename from src/ndv/_views/_pygfx/__init__.py rename to src/ndv/views/_pygfx/__init__.py diff --git a/src/ndv/_views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py similarity index 99% rename from src/ndv/_views/_pygfx/_array_canvas.py rename to src/ndv/views/_pygfx/_array_canvas.py index 97ee47b1..4f884145 100755 --- a/src/ndv/_views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -11,8 +11,8 @@ import pylinalg as la from ndv._types import CursorType -from ndv._views._app import filter_mouse_events -from ndv._views.bases import ArrayCanvas, CanvasElement, ImageHandle +from ndv.views._app import filter_mouse_events +from ndv.views.bases import ArrayCanvas, CanvasElement, ImageHandle if TYPE_CHECKING: from collections.abc import Sequence @@ -391,7 +391,7 @@ def cursor_at(self, canvas_pos: Sequence[float]) -> CursorType | None: def get_canvas_class() -> WgpuCanvas: - from ndv._views._app import GuiFrontend, gui_frontend + from ndv.views._app import GuiFrontend, gui_frontend frontend = gui_frontend() if frontend == GuiFrontend.QT: diff --git a/src/ndv/_views/_qt/__init__.py b/src/ndv/views/_qt/__init__.py similarity index 100% rename from src/ndv/_views/_qt/__init__.py rename to src/ndv/views/_qt/__init__.py diff --git a/src/ndv/_views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py similarity index 99% rename from src/ndv/_views/_qt/_array_view.py rename to src/ndv/views/_qt/_array_view.py index 3e8f880b..e4931ca5 100644 --- a/src/ndv/_views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -20,8 +20,8 @@ from superqt.iconify import QIconifyIcon from superqt.utils import signals_blocked -from ndv._views.bases import ArrayView, LutView from ndv.models._array_display_model import ChannelMode +from ndv.views.bases import ArrayView, LutView if TYPE_CHECKING: from collections.abc import Container, Hashable, Mapping, Sequence diff --git a/src/ndv/_views/_qt/_save_button.py b/src/ndv/views/_qt/_save_button.py similarity index 100% rename from src/ndv/_views/_qt/_save_button.py rename to src/ndv/views/_qt/_save_button.py diff --git a/src/ndv/_views/_qt/spin.gif b/src/ndv/views/_qt/spin.gif similarity index 100% rename from src/ndv/_views/_qt/spin.gif rename to src/ndv/views/_qt/spin.gif diff --git a/src/ndv/_views/_vispy/__init__.py b/src/ndv/views/_vispy/__init__.py similarity index 100% rename from src/ndv/_views/_vispy/__init__.py rename to src/ndv/views/_vispy/__init__.py diff --git a/src/ndv/_views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py similarity index 98% rename from src/ndv/_views/_vispy/_array_canvas.py rename to src/ndv/views/_vispy/_array_canvas.py index aac99b2a..e4496ce6 100755 --- a/src/ndv/_views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -15,10 +15,10 @@ from vispy.util.quaternion import Quaternion from ndv._types import CursorType -from ndv._views._app import filter_mouse_events -from ndv._views._vispy._utils import supports_float_textures -from ndv._views.bases import ArrayCanvas -from ndv._views.bases.graphics._canvas_elements import ( +from ndv.views._app import filter_mouse_events +from ndv.views._vispy._utils import supports_float_textures +from ndv.views.bases import ArrayCanvas +from ndv.views.bases.graphics._canvas_elements import ( CanvasElement, ImageHandle, RoiHandle, diff --git a/src/ndv/_views/_vispy/_histogram.py b/src/ndv/views/_vispy/_histogram.py similarity index 99% rename from src/ndv/_views/_vispy/_histogram.py rename to src/ndv/views/_vispy/_histogram.py index a0227fcc..1da63ad7 100644 --- a/src/ndv/_views/_vispy/_histogram.py +++ b/src/ndv/views/_vispy/_histogram.py @@ -7,8 +7,8 @@ from vispy import scene from ndv._types import CursorType -from ndv._views._app import filter_mouse_events -from ndv._views.bases import HistogramCanvas +from ndv.views._app import filter_mouse_events +from ndv.views.bases import HistogramCanvas from ._plot_widget import PlotWidget diff --git a/src/ndv/_views/_vispy/_plot_widget.py b/src/ndv/views/_vispy/_plot_widget.py similarity index 100% rename from src/ndv/_views/_vispy/_plot_widget.py rename to src/ndv/views/_vispy/_plot_widget.py diff --git a/src/ndv/_views/_vispy/_utils.py b/src/ndv/views/_vispy/_utils.py similarity index 100% rename from src/ndv/_views/_vispy/_utils.py rename to src/ndv/views/_vispy/_utils.py diff --git a/src/ndv/_views/_wx/__init__.py b/src/ndv/views/_wx/__init__.py similarity index 100% rename from src/ndv/_views/_wx/__init__.py rename to src/ndv/views/_wx/__init__.py diff --git a/src/ndv/_views/_wx/_array_view.py b/src/ndv/views/_wx/_array_view.py similarity index 98% rename from src/ndv/_views/_wx/_array_view.py rename to src/ndv/views/_wx/_array_view.py index d621a590..d2ce9f29 100644 --- a/src/ndv/_views/_wx/_array_view.py +++ b/src/ndv/views/_wx/_array_view.py @@ -7,10 +7,10 @@ import wx.lib.newevent from psygnal import Signal -from ndv._views._wx._labeled_slider import WxLabeledSlider -from ndv._views.bases._array_view import ArrayView -from ndv._views.bases._lut_view import LutView from ndv.models._array_display_model import ChannelMode +from ndv.views._wx._labeled_slider import WxLabeledSlider +from ndv.views.bases._array_view import ArrayView +from ndv.views.bases._lut_view import LutView from .range_slider import RangeSlider diff --git a/src/ndv/_views/_wx/_labeled_slider.py b/src/ndv/views/_wx/_labeled_slider.py similarity index 100% rename from src/ndv/_views/_wx/_labeled_slider.py rename to src/ndv/views/_wx/_labeled_slider.py diff --git a/src/ndv/_views/_wx/range_slider.py b/src/ndv/views/_wx/range_slider.py similarity index 100% rename from src/ndv/_views/_wx/range_slider.py rename to src/ndv/views/_wx/range_slider.py diff --git a/src/ndv/_views/bases/__init__.py b/src/ndv/views/bases/__init__.py similarity index 100% rename from src/ndv/_views/bases/__init__.py rename to src/ndv/views/bases/__init__.py diff --git a/src/ndv/_views/bases/_array_view.py b/src/ndv/views/bases/_array_view.py similarity index 94% rename from src/ndv/_views/bases/_array_view.py rename to src/ndv/views/bases/_array_view.py index d61ac3ed..984581db 100644 --- a/src/ndv/_views/bases/_array_view.py +++ b/src/ndv/views/bases/_array_view.py @@ -6,14 +6,14 @@ from psygnal import Signal -from ndv._views.bases._view_base import Viewable from ndv.models._array_display_model import ChannelMode +from ndv.views.bases._view_base import Viewable if TYPE_CHECKING: from collections.abc import Container, Hashable, Mapping, Sequence from ndv._types import AxisKey - from ndv._views.bases._lut_view import LutView + from ndv.views.bases._lut_view import LutView class ArrayView(Viewable): diff --git a/src/ndv/_views/bases/_lut_view.py b/src/ndv/views/bases/_lut_view.py similarity index 100% rename from src/ndv/_views/bases/_lut_view.py rename to src/ndv/views/bases/_lut_view.py diff --git a/src/ndv/_views/bases/_view_base.py b/src/ndv/views/bases/_view_base.py similarity index 100% rename from src/ndv/_views/bases/_view_base.py rename to src/ndv/views/bases/_view_base.py diff --git a/src/ndv/_views/bases/graphics/__init__.py b/src/ndv/views/bases/graphics/__init__.py similarity index 100% rename from src/ndv/_views/bases/graphics/__init__.py rename to src/ndv/views/bases/graphics/__init__.py diff --git a/src/ndv/_views/bases/graphics/_canvas.py b/src/ndv/views/bases/graphics/_canvas.py similarity index 93% rename from src/ndv/_views/bases/graphics/_canvas.py rename to src/ndv/views/bases/graphics/_canvas.py index 642227ff..d8bcc503 100644 --- a/src/ndv/_views/bases/graphics/_canvas.py +++ b/src/ndv/views/bases/graphics/_canvas.py @@ -5,8 +5,8 @@ import numpy as np -from ndv._views.bases._lut_view import LutView -from ndv._views.bases._view_base import Viewable +from ndv.views.bases._lut_view import LutView +from ndv.views.bases._view_base import Viewable from ._mouseable import Mouseable @@ -16,7 +16,7 @@ import cmap import numpy as np - from ndv._views.bases.graphics._canvas_elements import ( + from ndv.views.bases.graphics._canvas_elements import ( CanvasElement, ImageHandle, RoiHandle, diff --git a/src/ndv/_views/bases/graphics/_canvas_elements.py b/src/ndv/views/bases/graphics/_canvas_elements.py similarity index 98% rename from src/ndv/_views/bases/graphics/_canvas_elements.py rename to src/ndv/views/bases/graphics/_canvas_elements.py index 80a84b84..5225e9d0 100644 --- a/src/ndv/_views/bases/graphics/_canvas_elements.py +++ b/src/ndv/views/bases/graphics/_canvas_elements.py @@ -3,7 +3,7 @@ from abc import abstractmethod from typing import TYPE_CHECKING, Any -from ndv._views.bases._lut_view import LutView +from ndv.views.bases._lut_view import LutView from ._mouseable import Mouseable diff --git a/src/ndv/_views/bases/graphics/_mouseable.py b/src/ndv/views/bases/graphics/_mouseable.py similarity index 100% rename from src/ndv/_views/bases/graphics/_mouseable.py rename to src/ndv/views/bases/graphics/_mouseable.py diff --git a/tests/conftest.py b/tests/conftest.py index 7bd805e6..9e4162c9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,8 +11,8 @@ import pytest -from ndv._views import gui_frontend -from ndv._views._app import GuiFrontend +from ndv.views import gui_frontend +from ndv.views._app import GuiFrontend if TYPE_CHECKING: from asyncio import AbstractEventLoop diff --git a/tests/test_controller.py b/tests/test_controller.py index 32dad47d..48949c4b 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -9,14 +9,14 @@ import pytest from ndv._types import MouseMoveEvent -from ndv._views import _app, gui_frontend -from ndv._views.bases._array_view import ArrayView -from ndv._views.bases._lut_view import LutView -from ndv._views.bases.graphics._canvas import ArrayCanvas, HistogramCanvas -from ndv._views.bases.graphics._canvas_elements import ImageHandle from ndv.controllers import ArrayViewer from ndv.models._array_display_model import ArrayDisplayModel, ChannelMode from ndv.models._lut_model import LUTModel +from ndv.views import _app, gui_frontend +from ndv.views.bases._array_view import ArrayView +from ndv.views.bases._lut_view import LutView +from ndv.views.bases.graphics._canvas import ArrayCanvas, HistogramCanvas +from ndv.views.bases.graphics._canvas_elements import ImageHandle if TYPE_CHECKING: from ndv.controllers._channel_controller import ChannelController From 1a4107d6164e48f646845e21d9768b51ad2e49d9 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 08:24:12 -0600 Subject: [PATCH 03/33] Remove set_colormap_without_signal usage in tests --- tests/test_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_controller.py b/tests/test_controller.py index b30625e3..5252b460 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -103,7 +103,7 @@ def test_controller() -> None: # setting a new ArrayDisplay model updates the appropriate view widgets ch_ctrl = cast("ChannelController", ctrl._lut_controllers[None]) - ch_ctrl.lut_views[0].set_colormap_without_signal.reset_mock() + ch_ctrl.lut_views[0].set_colormap.reset_mock() ctrl.display_model = ArrayDisplayModel(default_lut=LUTModel(cmap="green")) # fails # ch_ctrl.lut_views[0].set_colormap_without_signal.assert_called_once() From 9abec153b01c88669320f56afaf338115dc765c4 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 12:33:03 -0600 Subject: [PATCH 04/33] Sync autoscale and clims --- src/ndv/views/_jupyter/_array_view.py | 18 ++++++++++++++---- src/ndv/views/_qt/_array_view.py | 10 +++++++--- src/ndv/views/_wx/_array_view.py | 1 + src/ndv/views/_wx/range_slider.py | 2 -- .../views/bases/graphics/_canvas_elements.py | 5 ++--- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/ndv/views/_jupyter/_array_view.py b/src/ndv/views/_jupyter/_array_view.py index 117ee2c7..65f3849d 100644 --- a/src/ndv/views/_jupyter/_array_view.py +++ b/src/ndv/views/_jupyter/_array_view.py @@ -55,23 +55,25 @@ def __init__(self) -> None: self._cmap.observe(self._on_cmap_changed, names="value") self._clims.observe(self._on_clims_changed, names="value") self._auto_clim.observe(self._on_autoscale_changed, names="value") + self._updating: bool = False # ------------------ emit changes to the controller ------------------ def _on_clims_changed(self, change: dict[str, Any]) -> None: - if self._model: + if self._model and not self._updating: self._model.clims = self._clims.value + self._model.autoscale = False def _on_visible_changed(self, change: dict[str, Any]) -> None: - if self._model: + if self._model and not self._updating: self._model.visible = self._visible.value def _on_cmap_changed(self, change: dict[str, Any]) -> None: - if self._model: + if self._model and not self._updating: self._model.cmap = self._cmap.value def _on_autoscale_changed(self, change: dict[str, Any]) -> None: - if self._model: + if self._model and not self._updating: self._model.autoscale = self._auto_clim.value # ------------------ receive changes from the controller --------------- @@ -84,19 +86,27 @@ def set_channel_name(self, name: str) -> None: def set_auto_scale(self, auto: bool) -> None: # with self.autoscaleChanged.blocked(): + self._updating = True self._auto_clim.value = auto + self._updating = False def set_colormap(self, cmap: cmap.Colormap) -> None: # with self.cmapChanged.blocked(): + self._updating = True self._cmap.value = cmap.name.split(":")[-1] # FIXME: this is a hack + self._updating = False def set_clims(self, clims: tuple[float, float]) -> None: # with self.climsChanged.blocked(): + self._updating = True self._clims.value = clims + self._updating = False def set_channel_visible(self, visible: bool) -> None: # with self.visibilityChanged.blocked(): + self._updating = True self._visible.value = visible + self._updating = False def set_gamma(self, gamma: float) -> None: pass diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index e4931ca5..a8e7dcc1 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -138,13 +138,16 @@ def set_channel_name(self, name: str) -> None: self._qwidget.visible.setText(name) def set_auto_scale(self, auto: bool) -> None: - self._qwidget.auto_clim.setChecked(auto) + with signals_blocked(self._qwidget.auto_clim): + self._qwidget.auto_clim.setChecked(auto) def set_colormap(self, cmap: cmap.Colormap) -> None: - self._qwidget.cmap.setCurrentColormap(cmap) + with signals_blocked(self._qwidget.cmap): + self._qwidget.cmap.setCurrentColormap(cmap) def set_clims(self, clims: tuple[float, float]) -> None: - self._qwidget.clims.setValue(clims) + with signals_blocked(self._qwidget.clims): + self._qwidget.clims.setValue(clims) def set_gamma(self, gamma: float) -> None: pass @@ -169,6 +172,7 @@ def _on_q_cmap_changed(self, cmap: cmap.Colormap) -> None: def _on_q_clims_changed(self, clims: tuple[float, float]) -> None: if self._model: self._model.clims = clims + self._model.autoscale = False def _on_q_auto_changed(self, autoscale: bool) -> None: if self._model: diff --git a/src/ndv/views/_wx/_array_view.py b/src/ndv/views/_wx/_array_view.py index cdf0fbc4..d144bd48 100644 --- a/src/ndv/views/_wx/_array_view.py +++ b/src/ndv/views/_wx/_array_view.py @@ -74,6 +74,7 @@ def _on_cmap_changed(self, event: wx.CommandEvent) -> None: def _on_clims_changed(self, event: wx.CommandEvent) -> None: if self._model: self._model.clims = self._wxwidget.clims.GetValues() + self._model.autoscale = False def _on_autoscale_changed(self, event: wx.CommandEvent) -> None: if self._model: diff --git a/src/ndv/views/_wx/range_slider.py b/src/ndv/views/_wx/range_slider.py index a6fad08c..f453b2e7 100644 --- a/src/ndv/views/_wx/range_slider.py +++ b/src/ndv/views/_wx/range_slider.py @@ -86,8 +86,6 @@ def GetValue(self) -> int: def SetValue(self, value: int) -> None: self.value = value - # Post event notifying that value changed - self.PostEvent() def PostEvent(self) -> None: event = wx.PyCommandEvent(wx.EVT_SLIDER.typeId, self.parent.GetId()) diff --git a/src/ndv/views/bases/graphics/_canvas_elements.py b/src/ndv/views/bases/graphics/_canvas_elements.py index 5225e9d0..a0fae39c 100644 --- a/src/ndv/views/bases/graphics/_canvas_elements.py +++ b/src/ndv/views/bases/graphics/_canvas_elements.py @@ -91,10 +91,9 @@ def set_channel_name(self, name: str) -> None: pass def set_auto_scale(self, checked: bool) -> None: - # TODO: Make this computation alter the slider... - if checked: + if checked and self.model: d = self.data() - self.set_clims((d.min(), d.max())) + self.model.clims = (d.min(), d.max()) def set_colormap(self, cmap: _cmap.Colormap) -> None: self.set_cmap(cmap) From f64bb06ccab3f2e90202ed2f6a53d1ed4100a2f4 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 13:20:51 -0600 Subject: [PATCH 05/33] ImageHandle.cmap -> ImageHandle.colormap Aligns with LutView --- src/ndv/v1/_old_viewer.py | 4 ++-- src/ndv/v1/_qt/_lut_control.py | 6 +++--- src/ndv/views/_pygfx/_array_canvas.py | 4 ++-- src/ndv/views/_vispy/_array_canvas.py | 4 ++-- src/ndv/views/bases/graphics/_canvas_elements.py | 11 +++-------- 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/ndv/v1/_old_viewer.py b/src/ndv/v1/_old_viewer.py index 15ee3b34..2155c4fe 100755 --- a/src/ndv/v1/_old_viewer.py +++ b/src/ndv/v1/_old_viewer.py @@ -574,11 +574,11 @@ def _update_canvas_data(self, data: np.ndarray, index: Indices) -> None: ) if datum.ndim == 2: handle = self._canvas.add_image(datum) - handle.set_cmap(cm) + handle.set_colormap(cm) handles.append(handle) elif datum.ndim == 3: handle = self._canvas.add_volume(datum) - handle.set_cmap(cm) + handle.set_colormap(cm) handles.append(handle) if imkey not in self._lut_ctrls: ch_index = index.get(self._channel_axis, 0) diff --git a/src/ndv/v1/_qt/_lut_control.py b/src/ndv/v1/_qt/_lut_control.py index 24d14f6d..ff3ac0de 100644 --- a/src/ndv/v1/_qt/_lut_control.py +++ b/src/ndv/v1/_qt/_lut_control.py @@ -52,7 +52,7 @@ def __init__( self._cmap = CmapCombo() self._cmap.currentColormapChanged.connect(self._on_cmap_changed) for handle in self._handles: - self._cmap.addColormap(handle.cmap()) + self._cmap.addColormap(handle.colormap()) for color in cmaplist: self._cmap.addColormap(color) @@ -110,7 +110,7 @@ def _on_visible_changed(self, visible: bool) -> None: def _on_cmap_changed(self, cmap: cmap.Colormap) -> None: for handle in self._handles: - handle.set_cmap(cmap) + handle.set_colormap(cmap) def update_autoscale(self) -> None: if ( @@ -139,5 +139,5 @@ def update_autoscale(self) -> None: def add_handle(self, handle: ImageHandle) -> None: self._handles.append(handle) - self._cmap.addColormap(handle.cmap()) + self._cmap.addColormap(handle.colormap()) self.update_autoscale() diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index 4f884145..732de203 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -80,10 +80,10 @@ def set_gamma(self, gamma: float) -> None: # self._render() warnings.warn("Gamma correction is not supported in pygfx", stacklevel=2) - def cmap(self) -> _cmap.Colormap: + def colormap(self) -> _cmap.Colormap: return self._cmap - def set_cmap(self, cmap: _cmap.Colormap) -> None: + def set_colormap(self, cmap: _cmap.Colormap) -> None: self._cmap = cmap self._material.map = cmap.to_pygfx() self._render() diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index e4496ce6..9d5e991d 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -301,10 +301,10 @@ def gamma(self) -> float: def set_gamma(self, gamma: float) -> None: self._visual.gamma = gamma - def cmap(self) -> _cmap.Colormap: + def colormap(self) -> _cmap.Colormap: return self._cmap # FIXME - def set_cmap(self, cmap: _cmap.Colormap) -> None: + def set_colormap(self, cmap: _cmap.Colormap) -> None: self._cmap = cmap self._visual.cmap = cmap.to_vispy() diff --git a/src/ndv/views/bases/graphics/_canvas_elements.py b/src/ndv/views/bases/graphics/_canvas_elements.py index a0fae39c..5d321e4e 100644 --- a/src/ndv/views/bases/graphics/_canvas_elements.py +++ b/src/ndv/views/bases/graphics/_canvas_elements.py @@ -76,13 +76,13 @@ def gamma(self) -> float: ... @abstractmethod def set_gamma(self, gamma: float) -> None: ... @abstractmethod - def cmap(self) -> _cmap.Colormap: ... + def colormap(self) -> _cmap.Colormap: ... @abstractmethod - def set_cmap(self, cmap: _cmap.Colormap) -> None: ... + def set_colormap(self, cmap: _cmap.Colormap) -> None: ... # -- LutView methods -- # def close(self) -> None: - pass + self.remove() def frontend_widget(self) -> Any: return None @@ -95,14 +95,9 @@ def set_auto_scale(self, checked: bool) -> None: d = self.data() self.model.clims = (d.min(), d.max()) - def set_colormap(self, cmap: _cmap.Colormap) -> None: - self.set_cmap(cmap) - def set_channel_visible(self, visible: bool) -> None: self.set_visible(visible) - # set_clims, set_gamma reused above - class RoiHandle(CanvasElement): @abstractmethod From ae478c3ec7ee4db0b20154ca30962dfe16638a55 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 13:27:35 -0600 Subject: [PATCH 06/33] PyGFXImageHandle.set_gamma: Allow gamma=1 --- src/ndv/views/_pygfx/_array_canvas.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index 732de203..f9e2af7d 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -78,7 +78,8 @@ def gamma(self) -> float: def set_gamma(self, gamma: float) -> None: # self._material.gamma = gamma # self._render() - warnings.warn("Gamma correction is not supported in pygfx", stacklevel=2) + if gamma != 1: + warnings.warn("Gamma correction is not supported in pygfx", stacklevel=2) def colormap(self) -> _cmap.Colormap: return self._cmap From 6abb551c3da61ad74d158ae732c2c2f12bb513fa Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 13:40:51 -0600 Subject: [PATCH 07/33] Clean up LutView Use Optional"[LutModel]" over "LutModel | None" for Python 3.9 Remove unused ctor --- src/ndv/views/bases/_lut_view.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ndv/views/bases/_lut_view.py b/src/ndv/views/bases/_lut_view.py index c82f7d2d..a81dd51a 100644 --- a/src/ndv/views/bases/_lut_view.py +++ b/src/ndv/views/bases/_lut_view.py @@ -1,10 +1,16 @@ +from __future__ import annotations + from abc import abstractmethod +from typing import TYPE_CHECKING -import cmap +from ._view_base import Viewable -from ndv.models._lut_model import LUTModel +if TYPE_CHECKING: + import cmap -from ._view_base import Viewable + from ndv.models._lut_model import LUTModel + + pass class LutView(Viewable): @@ -12,9 +18,6 @@ class LutView(Viewable): _model: LUTModel | None = None - def __init__(self, model: LUTModel | None = None) -> None: - self.model = model - @abstractmethod def set_channel_name(self, name: str) -> None: """Set the name of the channel to `name`.""" From be5b5812ef47b5253853463d981142df6f4ba5b0 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 14:08:54 -0600 Subject: [PATCH 08/33] Comment _channel_controller.py --- src/ndv/controllers/_channel_controller.py | 7 +------ src/ndv/views/bases/_lut_view.py | 4 ++++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index 72b8d014..eb3896f3 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -41,7 +41,7 @@ def add_lut_view(self, view: LutView) -> None: self.synchronize(view) def synchronize(self, *views: LutView) -> None: - """Make sure the view matches the model.""" + """Aligns all views against the backing model.""" _views: Iterable[LutView] = views or self.lut_views name = str(self.key) if self.key is not None else "" for view in _views: @@ -68,14 +68,9 @@ def update_texture_data(self, data: np.ndarray) -> None: def add_handle(self, handle: ImageHandle) -> None: """Add an image texture handle to the controller.""" - handle.model = self.lut_model self.handles.append(handle) self.add_lut_view(handle) - if self.lut_model.autoscale: - data = handle.data() - self.lut_model.clims = (data.min(), data.max()) - def get_value_at_index(self, idx: tuple[int, ...]) -> float | None: """Get the value of the data at the given index.""" if not (handles := self.handles): diff --git a/src/ndv/views/bases/_lut_view.py b/src/ndv/views/bases/_lut_view.py index a81dd51a..64f25889 100644 --- a/src/ndv/views/bases/_lut_view.py +++ b/src/ndv/views/bases/_lut_view.py @@ -57,12 +57,15 @@ def model(self) -> LUTModel | None: @model.setter def model(self, model: LUTModel | None) -> None: + # Disconnect old model if self._model is not None: self._model.events.autoscale.disconnect(self.set_auto_scale) self._model.events.clims.disconnect(self.set_clims) self._model.events.cmap.disconnect(self.set_colormap) self._model.events.gamma.disconnect(self.set_gamma) self._model.events.visible.disconnect(self.set_channel_visible) + + # Connect new model self._model = model if self._model is not None: self._model.events.autoscale.connect(self.set_auto_scale) @@ -74,6 +77,7 @@ def model(self, model: LUTModel | None) -> None: self.synchronize() def synchronize(self) -> None: + """Aligns the view against the backing model.""" if model := self._model: self.set_auto_scale(bool(model.autoscale)) if model.clims: From 14eb73112fb72f264f769518d6db29b8d818c06e Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 15:30:21 -0600 Subject: [PATCH 09/33] More cleanups --- src/ndv/controllers/_channel_controller.py | 1 - src/ndv/views/_jupyter/_array_view.py | 4 ---- 2 files changed, 5 deletions(-) diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index eb3896f3..e5654f59 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -38,7 +38,6 @@ def add_lut_view(self, view: LutView) -> None: """Add a LUT view to the controller.""" view.model = self.lut_model self.lut_views.append(view) - self.synchronize(view) def synchronize(self, *views: LutView) -> None: """Aligns all views against the backing model.""" diff --git a/src/ndv/views/_jupyter/_array_view.py b/src/ndv/views/_jupyter/_array_view.py index 30a80838..0d487f55 100644 --- a/src/ndv/views/_jupyter/_array_view.py +++ b/src/ndv/views/_jupyter/_array_view.py @@ -100,25 +100,21 @@ def set_channel_name(self, name: str) -> None: # to avoid loops, unnecessary updates, and unexpected behavior def set_auto_scale(self, auto: bool) -> None: - # with self.autoscaleChanged.blocked(): self._updating = True self._auto_clim.value = auto self._updating = False def set_colormap(self, cmap: cmap.Colormap) -> None: - # with self.cmapChanged.blocked(): self._updating = True self._cmap.value = cmap.name.split(":")[-1] # FIXME: this is a hack self._updating = False def set_clims(self, clims: tuple[float, float]) -> None: - # with self.climsChanged.blocked(): self._updating = True self._clims.value = clims self._updating = False def set_channel_visible(self, visible: bool) -> None: - # with self.visibilityChanged.blocked(): self._updating = True self._visible.value = visible self._updating = False From 1f7c95774d4c1f32651f6c0cadee82512d65da74 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 17:04:45 -0600 Subject: [PATCH 10/33] Add tests for QLutView --- src/ndv/views/_qt/_array_view.py | 3 ++ tests/views/qt/__init__.py | 8 ++++ tests/views/qt/test_lut_view.py | 79 ++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 tests/views/qt/__init__.py create mode 100644 tests/views/qt/test_lut_view.py diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index a8e7dcc1..843941e5 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -86,7 +86,10 @@ def setCurrentColormap(self, cmap_: cmap.Colormap) -> None: if item.name == cmap_.name: self.setCurrentIndex(idx) else: + # Add the colormap to the end and select it self.addColormap(cmap_) + # NB: Last item is "Add New...", new cmap is second-to-last + self.setCurrentIndex(self.count() - 2) class _QLUTWidget(QWidget): diff --git a/tests/views/qt/__init__.py b/tests/views/qt/__init__.py new file mode 100644 index 00000000..0b3ec0b2 --- /dev/null +++ b/tests/views/qt/__init__.py @@ -0,0 +1,8 @@ +"""Tests pertaining to Qt components""" + +from importlib.util import find_spec + +import pytest + +if find_spec("qtpy") is None: + pytest.skip("Skipping Qt tests as Qt is not installed") diff --git a/tests/views/qt/test_lut_view.py b/tests/views/qt/test_lut_view.py new file mode 100644 index 00000000..5fa1ee40 --- /dev/null +++ b/tests/views/qt/test_lut_view.py @@ -0,0 +1,79 @@ +from __future__ import annotations + +import cmap +from pytest import fixture + +from ndv.models._lut_model import LUTModel +from ndv.views._app import QtProvider +from ndv.views._qt._array_view import QLutView + + +@fixture(autouse=True) +def init_provider() -> None: + provider = QtProvider() + provider.create_app() + + +@fixture +def model() -> LUTModel: + return LUTModel() + + +@fixture +def view(model: LUTModel) -> QLutView: + view = QLutView() + # Set the model + assert view.model is None + view.model = model + assert view.model is model + return view + + +def test_QLutView_update_model(model: LUTModel, view: QLutView) -> None: + """Ensures the view updates when the model is changed.""" + + new_clims = (4, 5) + assert view._qwidget.clims.value() != new_clims + model.clims = new_clims + assert view._qwidget.clims.value() == new_clims + + new_visible = not model.visible + model.visible = new_visible + assert view._qwidget.visible.isChecked() == new_visible + + new_cmap = cmap.Colormap("red") + assert view._qwidget.cmap.currentColormap() != new_cmap + model.cmap = new_cmap + assert view._qwidget.cmap.currentColormap() == new_cmap + + new_autoscale = not model.autoscale + model.autoscale = new_autoscale + assert view._qwidget.auto_clim.isChecked() == new_autoscale + + +def test_QLutView_update_view(model: LUTModel, view: QLutView) -> None: + """Ensures the model updates when the view is changed.""" + + new_clims = (5, 6) + assert model.clims != new_clims + view._qwidget.clims.setValue(new_clims) + assert model.clims == new_clims + + new_visible = not model.visible + view._qwidget.visible.setChecked(new_visible) + assert view._qwidget.visible.isChecked() == new_visible + + new_cmap = view._qwidget.cmap.itemColormap(1) + assert model.cmap != new_cmap + assert new_cmap is not None + view._qwidget.cmap.setCurrentIndex(1) + assert model.cmap == new_cmap + + new_autoscale = not model.autoscale + view._qwidget.auto_clim.setChecked(new_autoscale) + assert model.autoscale == new_autoscale + + # When gui clims change, autoscale should be disabled + model.autoscale = True + view._qwidget.clims.setValue((0, 1)) + assert model.autoscale is False From 8f82dd05b06ee97c32fd4068b2b3a5f33e4fc0f9 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 17:16:37 -0600 Subject: [PATCH 11/33] Fix qt check --- tests/views/qt/__init__.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/views/qt/__init__.py b/tests/views/qt/__init__.py index 0b3ec0b2..2eb6f427 100644 --- a/tests/views/qt/__init__.py +++ b/tests/views/qt/__init__.py @@ -1,8 +1,11 @@ """Tests pertaining to Qt components""" -from importlib.util import find_spec - import pytest -if find_spec("qtpy") is None: +try: + # NB: We could use importlib, but we'd have to search for qtpy + # AND for one of the many bindings (PyQt5, PyQt6, PySide2) + # This seems easier. + from qtpy.QtCore import Qt # noqa: F401 +except ImportError: pytest.skip("Skipping Qt tests as Qt is not installed") From f9c04ed2bb6e40a2ca7ce4e051d64fe0bf5c1d9a Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 13 Jan 2025 17:22:21 -0600 Subject: [PATCH 12/33] Fix pytest qt skip :) --- tests/views/qt/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/views/qt/__init__.py b/tests/views/qt/__init__.py index 2eb6f427..d3606260 100644 --- a/tests/views/qt/__init__.py +++ b/tests/views/qt/__init__.py @@ -8,4 +8,4 @@ # This seems easier. from qtpy.QtCore import Qt # noqa: F401 except ImportError: - pytest.skip("Skipping Qt tests as Qt is not installed") + pytest.skip("Skipping Qt tests as Qt is not installed", allow_module_level=True) From 53ef36ae15aa02cb967ac94e7942fdfed22478ec Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 14 Jan 2025 15:00:28 -0600 Subject: [PATCH 13/33] Add wx to dev extra --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index fa2da5a9..6a01dfcd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -73,7 +73,7 @@ docs = [ "mike==2.1.3", ] dev = [ - "ndv[test,vispy,pygfx,pyqt,jupyter]", + "ndv[test,vispy,pygfx,pyqt,jupyter,wxpython]", "pytest-qt", "ipython", "mypy", From 24fbc4560af5f4fcfe55049b0f52669f48596f9f Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 14 Jan 2025 15:06:40 -0600 Subject: [PATCH 14/33] Add LutView tests for Jupyter, Wx --- tests/views/jupyter/__init__.py | 10 +++ tests/views/jupyter/test_lut_view.py | 72 ++++++++++++++++ tests/views/wx_frontend/__init__.py | 8 ++ tests/views/wx_frontend/test_lut_view.py | 103 +++++++++++++++++++++++ 4 files changed, 193 insertions(+) create mode 100644 tests/views/jupyter/__init__.py create mode 100644 tests/views/jupyter/test_lut_view.py create mode 100644 tests/views/wx_frontend/__init__.py create mode 100644 tests/views/wx_frontend/test_lut_view.py diff --git a/tests/views/jupyter/__init__.py b/tests/views/jupyter/__init__.py new file mode 100644 index 00000000..7a1dcc4a --- /dev/null +++ b/tests/views/jupyter/__init__.py @@ -0,0 +1,10 @@ +"""Tests pertaining to Qt components""" + +from importlib.util import find_spec + +import pytest + +if not find_spec("ipywidgets"): + pytest.skip( + "Skipping Jupyter tests as Jupyter is not installed", allow_module_level=True + ) diff --git a/tests/views/jupyter/test_lut_view.py b/tests/views/jupyter/test_lut_view.py new file mode 100644 index 00000000..f57a1941 --- /dev/null +++ b/tests/views/jupyter/test_lut_view.py @@ -0,0 +1,72 @@ +from __future__ import annotations + +import cmap +from pytest import fixture + +from ndv.models._lut_model import LUTModel +from ndv.views._jupyter._array_view import JupyterLutView + + +@fixture +def model() -> LUTModel: + return LUTModel() + + +@fixture +def view(model: LUTModel) -> JupyterLutView: + view = JupyterLutView() + # Set the model + assert view.model is None + view.model = model + assert view.model is model + return view + + +def test_JupyterLutView_update_model(model: LUTModel, view: JupyterLutView) -> None: + """Ensures the view updates when the model is changed.""" + + new_clims = (4, 5) + assert view._clims.value != new_clims + model.clims = new_clims + assert view._clims.value == new_clims + + new_visible = not model.visible + model.visible = new_visible + assert view._visible.value == new_visible + + new_cmap = cmap.Colormap("red") + new_name = new_cmap.name.split(":")[-1] + assert view._cmap.value != new_name + model.cmap = new_cmap + assert view._cmap.value == new_name + + new_autoscale = not model.autoscale + model.autoscale = new_autoscale + assert view._auto_clim.value == new_autoscale + + +def test_JupyterLutView_update_view(model: LUTModel, view: JupyterLutView) -> None: + """Ensures the model updates when the view is changed.""" + + new_clims = (5, 6) + assert model.clims != new_clims + view._clims.value = new_clims + assert model.clims == new_clims + + new_visible = not model.visible + view._visible.value = new_visible + assert view._visible.value == new_visible + + new_cmap = view._cmap.options[1] + assert model.cmap != new_cmap + view._cmap.value = new_cmap + assert model.cmap == new_cmap + + new_autoscale = not model.autoscale + view._auto_clim.value = new_autoscale + assert model.autoscale == new_autoscale + + # When gui clims change, autoscale should be disabled + model.autoscale = True + view._clims.value = (0, 1) + assert model.autoscale is False diff --git a/tests/views/wx_frontend/__init__.py b/tests/views/wx_frontend/__init__.py new file mode 100644 index 00000000..ef4867c6 --- /dev/null +++ b/tests/views/wx_frontend/__init__.py @@ -0,0 +1,8 @@ +"""Tests pertaining to Qt components""" + +from importlib.util import find_spec + +import pytest + +if not find_spec("wx"): + pytest.skip("Skipping wx tests as wx is not installed", allow_module_level=True) diff --git a/tests/views/wx_frontend/test_lut_view.py b/tests/views/wx_frontend/test_lut_view.py new file mode 100644 index 00000000..f45f5520 --- /dev/null +++ b/tests/views/wx_frontend/test_lut_view.py @@ -0,0 +1,103 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import cmap +import wx +from pytest import fixture + +from ndv.models._lut_model import LUTModel +from ndv.views._app import WxProvider +from ndv.views._wx._array_view import WxLutView + +if TYPE_CHECKING: + from collections.abc import Generator + + +@fixture(autouse=True) +def app() -> Generator[None, None, None]: + # Create wx app + provider = WxProvider() + provider.create_app() + # NB: Keep app alive during test + yield + return + + +@fixture +def model() -> LUTModel: + return LUTModel() + + +@fixture +def view(model: LUTModel) -> WxLutView: + frame = wx.Frame(None) + view = WxLutView(frame) + assert view.model is None + view.model = model + assert view.model is model + return view + + +def test_WxLutView_update_model(model: LUTModel, view: WxLutView) -> None: + """Ensures the view updates when the model is changed.""" + + new_clims = (4, 5) + assert view._wxwidget.clims.GetValues() != new_clims + model.clims = new_clims + assert view._wxwidget.clims.GetValues() == new_clims + + new_visible = not model.visible + model.visible = new_visible + assert view._wxwidget.visible.GetValue() == new_visible + + new_cmap = cmap.Colormap("red") + assert view._wxwidget.cmap.GetValue() != new_cmap + model.cmap = new_cmap + assert view._wxwidget.cmap.GetValue() == new_cmap + + new_autoscale = not model.autoscale + model.autoscale = new_autoscale + assert view._wxwidget.auto_clim.GetValue() == new_autoscale + + +def test_WxLutView_update_view(model: LUTModel, view: WxLutView) -> None: + """Ensures the model updates when the view is changed.""" + + new_clims = (5, 6) + assert model.clims != new_clims + view._wxwidget.clims.SetValue(*new_clims) + ev = wx.PyCommandEvent(wx.EVT_SLIDER.typeId, view._wxwidget.clims.GetId()) + wx.PostEvent(view._wxwidget.clims.GetEventHandler(), ev) + wx.Yield() + assert model.clims == new_clims + + new_visible = not model.visible + view._wxwidget.visible.SetValue(new_visible) + ev = wx.PyCommandEvent(wx.EVT_CHECKBOX.typeId, view._wxwidget.visible.GetId()) + wx.PostEvent(view._wxwidget.visible.GetEventHandler(), ev) + wx.Yield() + assert model.visible == new_visible + + new_cmap = cmap.Colormap("red") + assert model.cmap != new_cmap + view._wxwidget.cmap.SetValue(new_cmap.name) + ev = wx.PyCommandEvent(wx.EVT_COMBOBOX.typeId, view._wxwidget.cmap.GetId()) + wx.PostEvent(view._wxwidget.cmap.GetEventHandler(), ev) + wx.Yield() + assert model.cmap == new_cmap + + new_autoscale = not model.autoscale + view._wxwidget.auto_clim.SetValue(new_autoscale) + ev = wx.PyCommandEvent(wx.EVT_TOGGLEBUTTON.typeId, view._wxwidget.auto_clim.GetId()) + wx.PostEvent(view._wxwidget.auto_clim.GetEventHandler(), ev) + wx.Yield() + assert model.autoscale == new_autoscale + + # When gui clims change, autoscale should be disabled + model.autoscale = True + view._wxwidget.clims.SetValue(0, 1) + ev = wx.PyCommandEvent(wx.EVT_SLIDER.typeId, view._wxwidget.clims.GetId()) + wx.PostEvent(view._wxwidget.clims.GetEventHandler(), ev) + wx.Yield() + assert model.autoscale is False From 617f77c65a690d1f1990db66383990c186c86e86 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 14 Jan 2025 15:44:28 -0600 Subject: [PATCH 15/33] Improve wx tests --- tests/views/wx_frontend/test_lut_view.py | 62 +++++++++++------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/tests/views/wx_frontend/test_lut_view.py b/tests/views/wx_frontend/test_lut_view.py index f45f5520..fd1d3ba5 100644 --- a/tests/views/wx_frontend/test_lut_view.py +++ b/tests/views/wx_frontend/test_lut_view.py @@ -1,7 +1,5 @@ from __future__ import annotations -from typing import TYPE_CHECKING - import cmap import wx from pytest import fixture @@ -10,18 +8,12 @@ from ndv.views._app import WxProvider from ndv.views._wx._array_view import WxLutView -if TYPE_CHECKING: - from collections.abc import Generator - -@fixture(autouse=True) -def app() -> Generator[None, None, None]: +@fixture(autouse=True, scope="module") +def app() -> wx.App: # Create wx app provider = WxProvider() - provider.create_app() - # NB: Keep app alive during test - yield - return + return provider.create_app() @fixture @@ -30,7 +22,8 @@ def model() -> LUTModel: @fixture -def view(model: LUTModel) -> WxLutView: +def view(app: wx.App, model: LUTModel) -> WxLutView: + # NB: wx.App necessary although unused frame = wx.Frame(None) view = WxLutView(frame) assert view.model is None @@ -61,43 +54,46 @@ def test_WxLutView_update_model(model: LUTModel, view: WxLutView) -> None: assert view._wxwidget.auto_clim.GetValue() == new_autoscale -def test_WxLutView_update_view(model: LUTModel, view: WxLutView) -> None: +def test_WxLutView_update_view(app: wx.App, model: LUTModel, view: WxLutView) -> None: """Ensures the model updates when the view is changed.""" + def processEvent(evt: wx.PyEventBinder, wdg: wx.Control) -> None: + ev = wx.PyCommandEvent(evt.typeId, wdg.GetId()) + wx.PostEvent(wdg.GetEventHandler(), ev) + # Borrowed from: + # https://github.com/wxWidgets/Phoenix/blob/master/unittests/wtc.py#L41 + evtLoop = app.GetTraits().CreateEventLoop() + wx.EventLoopActivator(evtLoop) + evtLoop.YieldFor(wx.EVT_CATEGORY_ALL) + new_clims = (5, 6) assert model.clims != new_clims - view._wxwidget.clims.SetValue(*new_clims) - ev = wx.PyCommandEvent(wx.EVT_SLIDER.typeId, view._wxwidget.clims.GetId()) - wx.PostEvent(view._wxwidget.clims.GetEventHandler(), ev) - wx.Yield() + clim_wdg = view._wxwidget.clims + clim_wdg.SetValue(*new_clims) + processEvent(wx.EVT_SLIDER, clim_wdg) assert model.clims == new_clims new_visible = not model.visible - view._wxwidget.visible.SetValue(new_visible) - ev = wx.PyCommandEvent(wx.EVT_CHECKBOX.typeId, view._wxwidget.visible.GetId()) - wx.PostEvent(view._wxwidget.visible.GetEventHandler(), ev) - wx.Yield() + vis_wdg = view._wxwidget.visible + vis_wdg.SetValue(new_visible) + processEvent(wx.EVT_CHECKBOX, vis_wdg) assert model.visible == new_visible new_cmap = cmap.Colormap("red") assert model.cmap != new_cmap - view._wxwidget.cmap.SetValue(new_cmap.name) - ev = wx.PyCommandEvent(wx.EVT_COMBOBOX.typeId, view._wxwidget.cmap.GetId()) - wx.PostEvent(view._wxwidget.cmap.GetEventHandler(), ev) - wx.Yield() + cmap_wdg = view._wxwidget.cmap + cmap_wdg.SetValue(new_cmap.name) + processEvent(wx.EVT_COMBOBOX, cmap_wdg) assert model.cmap == new_cmap new_autoscale = not model.autoscale - view._wxwidget.auto_clim.SetValue(new_autoscale) - ev = wx.PyCommandEvent(wx.EVT_TOGGLEBUTTON.typeId, view._wxwidget.auto_clim.GetId()) - wx.PostEvent(view._wxwidget.auto_clim.GetEventHandler(), ev) - wx.Yield() + auto_wdg = view._wxwidget.auto_clim + auto_wdg.SetValue(new_autoscale) + processEvent(wx.EVT_TOGGLEBUTTON, auto_wdg) assert model.autoscale == new_autoscale # When gui clims change, autoscale should be disabled model.autoscale = True - view._wxwidget.clims.SetValue(0, 1) - ev = wx.PyCommandEvent(wx.EVT_SLIDER.typeId, view._wxwidget.clims.GetId()) - wx.PostEvent(view._wxwidget.clims.GetEventHandler(), ev) - wx.Yield() + clim_wdg.SetValue(0, 1) + processEvent(wx.EVT_SLIDER, clim_wdg) assert model.autoscale is False From 60ac45678cd8a7089375b3cda3186eed61b3268f Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 14 Jan 2025 16:49:02 -0600 Subject: [PATCH 16/33] Move autoscale clims to controller Since the actual value to set here is gathered from the data, not from the user, it shouldn't really be on the view. Also will make things much easier when multiple handles have different data. --- src/ndv/controllers/_channel_controller.py | 23 +++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index e5654f59..3db268d5 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -30,6 +30,7 @@ def __init__( self.key = key self.lut_views: list[LutView] = [] self.lut_model = lut_model + self.lut_model.events.autoscale.connect(self._auto_scale) self.handles: list[ImageHandle] = [] for v in views: self.add_lut_view(v) @@ -57,13 +58,14 @@ def update_texture_data(self, data: np.ndarray) -> None: handles[0].set_data(data) # if this image handle is visible and autoscale is on, then we need # to update the clim values - if self.lut_model.autoscale: - self.lut_model.clims = (data.min(), data.max()) - # lut_view.setClims((data.min(), data.max())) - # technically... the LutView may also emit a signal that the - # controller listens to, and then updates the image handle - # but this next line is more direct - # self._handles[None].clim = (data.min(), data.max()) + self._auto_scale() + # if self.lut_model.autoscale: + # self.lut_model.clims = (data.min(), data.max()) + # lut_view.setClims((data.min(), data.max())) + # technically... the LutView may also emit a signal that the + # controller listens to, and then updates the image handle + # but this next line is more direct + # self._handles[None].clim = (data.min(), data.max()) def add_handle(self, handle: ImageHandle) -> None: """Add an image texture handle to the controller.""" @@ -86,3 +88,10 @@ def get_value_at_index(self, idx: tuple[int, ...]) -> float | None: # the data source directly. return handle.data()[idx] # type: ignore [no-any-return] return None + + def _auto_scale(self) -> None: + if self.lut_model.autoscale and len(self.handles): + self.lut_model.clims = ( + min([handle.data().min() for handle in self.handles]), + max([handle.data().max() for handle in self.handles]), + ) From 16237f207ab953fcd43593d3dd944614b47aceb2 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 14 Jan 2025 16:55:55 -0600 Subject: [PATCH 17/33] Fix _CmapCombo.setCurrentColormap --- src/ndv/views/_qt/_array_view.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index 843941e5..037104d2 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -84,12 +84,15 @@ def setCurrentColormap(self, cmap_: cmap.Colormap) -> None: for idx in range(self.count()): if item := self.itemColormap(idx): if item.name == cmap_.name: + # cmap_ is already here - just select it self.setCurrentIndex(idx) - else: - # Add the colormap to the end and select it - self.addColormap(cmap_) - # NB: Last item is "Add New...", new cmap is second-to-last - self.setCurrentIndex(self.count() - 2) + return + + # cmap_ not in the combo box - add it! + self.addColormap(cmap_) + # then, select it! + # NB: "Add..." was at idx, now it's at idx+1 and cmap_ is at idx + self.setCurrentIndex(idx) class _QLUTWidget(QWidget): From 32b578aad26cb2d99451542e5aa89ffa78576152 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 14 Jan 2025 17:29:54 -0600 Subject: [PATCH 18/33] Test enabling autoscaling --- tests/test_controller.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_controller.py b/tests/test_controller.py index 5252b460..31a4c130 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -179,3 +179,23 @@ def test_array_viewer_with_app() -> None: index_mock.reset_mock() viewer._view.set_current_index(index) index_mock.assert_not_called() + + +@pytest.mark.usefixtures("any_app") +def test_channel_autoscale() -> None: + data = np.random.randint(0, 255, size=(10, 10, 10, 10, 10), dtype="uint8") + viewer = ArrayViewer(data) + + lut_model = viewer._lut_controllers[None].lut_model + old_clims = lut_model.clims + + # Adjust the clims with autoscale off + lut_model.autoscale = False + lut_model.clims = (old_clims[0] + 1, old_clims[1] + 1) + + # Assert turning autoscale back on reverts the clims + lut_model.autoscale = True + assert lut_model.clims == old_clims + + # NB: The view is (currently) responsible for disabling autoscale when + # it moves a clim. Thus that behavior is tested for each backend. From 409c89bb253cd8506c4b57a579de7061112c9c31 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 16 Jan 2025 18:39:13 -0600 Subject: [PATCH 19/33] Fixes --- src/ndv/controllers/_channel_controller.py | 3 ++- src/ndv/views/_jupyter/_array_view.py | 6 +++--- src/ndv/views/_qt/_array_view.py | 12 ++++++++---- src/ndv/views/_vispy/_histogram.py | 4 ++-- src/ndv/views/_wx/_array_view.py | 6 +++--- src/ndv/views/bases/_graphics/_canvas_elements.py | 3 ++- src/ndv/views/bases/_lut_view.py | 10 +++++----- 7 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index 0ca58249..3e800a75 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -41,6 +41,8 @@ def add_lut_view(self, view: LutView) -> None: """Add a LUT view to the controller.""" view.model = self.lut_model self.lut_views.append(view) + # TODO: Could probably reuse cached clims + self._auto_scale() def synchronize(self, *views: LutView) -> None: """Aligns all views against the backing model.""" @@ -65,7 +67,6 @@ def add_handle(self, handle: ImageHandle) -> None: """Add an image texture handle to the controller.""" self.handles.append(handle) self.add_lut_view(handle) - self._auto_scale() def get_value_at_index(self, idx: tuple[int, ...]) -> float | None: """Get the value of the data at the given index.""" diff --git a/src/ndv/views/_jupyter/_array_view.py b/src/ndv/views/_jupyter/_array_view.py index faf2d0d0..73e69c1a 100644 --- a/src/ndv/views/_jupyter/_array_view.py +++ b/src/ndv/views/_jupyter/_array_view.py @@ -6,7 +6,7 @@ import ipywidgets as widgets from ndv.models._array_display_model import ChannelMode -from ndv.models._lut_model import ClimsManual, ClimsMinMax +from ndv.models._lut_model import ClimPolicy, ClimsManual, ClimsMinMax from ndv.views.bases import ArrayView, LutView if TYPE_CHECKING: @@ -101,9 +101,9 @@ def set_channel_name(self, name: str) -> None: # NOTE: it's important to block signals when setting values from the controller # to avoid loops, unnecessary updates, and unexpected behavior - def set_auto_scale(self, auto: bool) -> None: + def set_clim_policy(self, policy: ClimPolicy) -> None: self._updating = True - self._auto_clim.value = auto + self._auto_clim.value = not policy.is_manual self._updating = False def set_colormap(self, cmap: cmap.Colormap) -> None: diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index 3607046f..ece719d6 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -21,7 +21,7 @@ from superqt.utils import signals_blocked from ndv.models._array_display_model import ChannelMode -from ndv.models._lut_model import ClimsManual, ClimsMinMax +from ndv.models._lut_model import ClimPolicy, ClimsManual, ClimsMinMax from ndv.views.bases import ArrayView, LutView if TYPE_CHECKING: @@ -153,9 +153,9 @@ def frontend_widget(self) -> QWidget: def set_channel_name(self, name: str) -> None: self._qwidget.visible.setText(name) - def set_auto_scale(self, auto: bool) -> None: + def set_clim_policy(self, policy: ClimPolicy) -> None: with signals_blocked(self._qwidget.auto_clim): - self._qwidget.auto_clim.setChecked(auto) + self._qwidget.auto_clim.setChecked(not policy.is_manual) def set_colormap(self, cmap: cmap.Colormap) -> None: with signals_blocked(self._qwidget.cmap): @@ -193,7 +193,11 @@ def _on_q_clims_changed(self, clims: tuple[float, float]) -> None: def _on_q_auto_changed(self, autoscale: bool) -> None: if self._model: - self._model.clims = ClimsMinMax() + if autoscale: + self._model.clims = ClimsMinMax() + else: + clims = self._qwidget.clims.value() + self._model.clims = ClimsManual(min=clims[0], max=clims[1]) class _QDimsSliders(QWidget): diff --git a/src/ndv/views/_vispy/_histogram.py b/src/ndv/views/_vispy/_histogram.py index ce2cce14..4e09f49a 100644 --- a/src/ndv/views/_vispy/_histogram.py +++ b/src/ndv/views/_vispy/_histogram.py @@ -7,7 +7,7 @@ from vispy import scene from ndv._types import CursorType -from ndv.models._lut_model import ClimsManual +from ndv.models._lut_model import ClimPolicy, ClimsManual from ndv.views._app import filter_mouse_events from ndv.views.bases import HistogramCanvas @@ -143,7 +143,7 @@ def set_clims(self, clims: tuple[float, float]) -> None: self._clims = clims self._update_lut_lines() - def set_auto_scale(self, autoscale: bool) -> None: + def set_clim_policy(self, policy: ClimPolicy) -> None: # Nothing to do (yet) pass diff --git a/src/ndv/views/_wx/_array_view.py b/src/ndv/views/_wx/_array_view.py index 71365bd2..325a9c9a 100644 --- a/src/ndv/views/_wx/_array_view.py +++ b/src/ndv/views/_wx/_array_view.py @@ -8,7 +8,7 @@ from psygnal import Signal from ndv.models._array_display_model import ChannelMode -from ndv.models._lut_model import ClimsManual, ClimsMinMax +from ndv.models._lut_model import ClimPolicy, ClimsManual, ClimsMinMax from ndv.views._wx._labeled_slider import WxLabeledSlider from ndv.views.bases import ArrayView, LutView @@ -89,8 +89,8 @@ def frontend_widget(self) -> wx.Window: def set_channel_name(self, name: str) -> None: self._wxwidget.visible.SetLabel(name) - def set_auto_scale(self, auto: bool) -> None: - self._wxwidget.auto_clim.SetValue(auto) + def set_clim_policy(self, policy: ClimPolicy) -> None: + self._wxwidget.auto_clim.SetValue(not policy.is_manual) def set_colormap(self, cmap: cmap.Colormap) -> None: name = cmap.name.split(":")[-1] # FIXME: this is a hack diff --git a/src/ndv/views/bases/_graphics/_canvas_elements.py b/src/ndv/views/bases/_graphics/_canvas_elements.py index 693ac16d..ebeac28b 100644 --- a/src/ndv/views/bases/_graphics/_canvas_elements.py +++ b/src/ndv/views/bases/_graphics/_canvas_elements.py @@ -14,6 +14,7 @@ import numpy as np from ndv._types import CursorType + from ndv.models._lut_model import ClimPolicy class CanvasElement(Mouseable): @@ -90,7 +91,7 @@ def frontend_widget(self) -> Any: def set_channel_name(self, name: str) -> None: pass - def set_auto_scale(self, checked: bool) -> None: + def set_clim_policy(self, policy: ClimPolicy) -> None: pass # if checked and self.model: diff --git a/src/ndv/views/bases/_lut_view.py b/src/ndv/views/bases/_lut_view.py index 0892772f..b5668353 100644 --- a/src/ndv/views/bases/_lut_view.py +++ b/src/ndv/views/bases/_lut_view.py @@ -8,7 +8,7 @@ if TYPE_CHECKING: import cmap - from ndv.models._lut_model import LUTModel + from ndv.models._lut_model import ClimPolicy, LUTModel pass @@ -23,7 +23,7 @@ def set_channel_name(self, name: str) -> None: """Set the name of the channel to `name`.""" @abstractmethod - def set_auto_scale(self, checked: bool) -> None: + def set_clim_policy(self, policy: ClimPolicy) -> None: """Set the autoscale button to checked if `checked` is True.""" @abstractmethod @@ -59,7 +59,7 @@ def model(self) -> LUTModel | None: def model(self, model: LUTModel | None) -> None: # Disconnect old model if self._model is not None: - self._model.events.clims.disconnect(self.set_clims) + self._model.events.clims.disconnect(self.set_clim_policy) self._model.events.cmap.disconnect(self.set_colormap) self._model.events.gamma.disconnect(self.set_gamma) self._model.events.visible.disconnect(self.set_channel_visible) @@ -67,7 +67,7 @@ def model(self, model: LUTModel | None) -> None: # Connect new model self._model = model if self._model is not None: - self._model.events.clims.connect(self.set_clims) + self._model.events.clims.connect(self.set_clim_policy) self._model.events.cmap.connect(self.set_colormap) self._model.events.gamma.connect(self.set_gamma) self._model.events.visible.connect(self.set_channel_visible) @@ -77,7 +77,7 @@ def model(self, model: LUTModel | None) -> None: def synchronize(self) -> None: """Aligns the view against the backing model.""" if model := self._model: - self.set_auto_scale(not model.clims.is_manual) + self.set_clim_policy(model.clims) self.set_colormap(model.cmap) self.set_gamma(model.gamma) self.set_channel_visible(model.visible) From c3498fdb84dd62161e1ad8c22401dab78e89899b Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 17 Jan 2025 11:28:39 -0600 Subject: [PATCH 20/33] Fix tests --- src/ndv/views/_jupyter/_array_view.py | 6 +++- src/ndv/views/_wx/_array_view.py | 6 +++- tests/views/jupyter/test_lut_view.py | 30 +++++++------------ tests/views/qt/test_lut_view.py | 34 ++++++++++----------- tests/views/wx_frontend/test_lut_view.py | 38 +++++++++++------------- 5 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/ndv/views/_jupyter/_array_view.py b/src/ndv/views/_jupyter/_array_view.py index 73e69c1a..b719b9f0 100644 --- a/src/ndv/views/_jupyter/_array_view.py +++ b/src/ndv/views/_jupyter/_array_view.py @@ -91,7 +91,11 @@ def _on_cmap_changed(self, change: dict[str, Any]) -> None: def _on_autoscale_changed(self, change: dict[str, Any]) -> None: if self._model and not self._updating: - self._model.clims = ClimsMinMax() + if change["new"]: # Autoscale + self._model.clims = ClimsMinMax() + else: # Manually scale + clims = self._clims.value + self._model.clims = ClimsManual(min=clims[0], max=clims[1]) # ------------------ receive changes from the controller --------------- diff --git a/src/ndv/views/_wx/_array_view.py b/src/ndv/views/_wx/_array_view.py index 325a9c9a..e13da301 100644 --- a/src/ndv/views/_wx/_array_view.py +++ b/src/ndv/views/_wx/_array_view.py @@ -80,7 +80,11 @@ def _on_clims_changed(self, event: wx.CommandEvent) -> None: def _on_autoscale_changed(self, event: wx.CommandEvent) -> None: if self._model: - self._model.clims = ClimsMinMax() + if self._wxwidget.auto_clim.GetValue(): + self._model.clims = ClimsMinMax() + else: # Manually scale + clims = self._wxwidget.clims.GetValues() + self._model.clims = ClimsManual(min=clims[0], max=clims[1]) # Public Methods def frontend_widget(self) -> wx.Window: diff --git a/tests/views/jupyter/test_lut_view.py b/tests/views/jupyter/test_lut_view.py index f57a1941..5ee9d0d6 100644 --- a/tests/views/jupyter/test_lut_view.py +++ b/tests/views/jupyter/test_lut_view.py @@ -3,7 +3,7 @@ import cmap from pytest import fixture -from ndv.models._lut_model import LUTModel +from ndv.models._lut_model import ClimsManual, ClimsMinMax, LUTModel from ndv.views._jupyter._array_view import JupyterLutView @@ -25,10 +25,10 @@ def view(model: LUTModel) -> JupyterLutView: def test_JupyterLutView_update_model(model: LUTModel, view: JupyterLutView) -> None: """Ensures the view updates when the model is changed.""" - new_clims = (4, 5) - assert view._clims.value != new_clims - model.clims = new_clims - assert view._clims.value == new_clims + auto_scale = not model.clims.is_manual + assert view._auto_clim.value == auto_scale + model.clims = ClimsManual(min=0, max=1) if auto_scale else ClimsMinMax() + assert view._auto_clim.value != auto_scale new_visible = not model.visible model.visible = new_visible @@ -40,19 +40,10 @@ def test_JupyterLutView_update_model(model: LUTModel, view: JupyterLutView) -> N model.cmap = new_cmap assert view._cmap.value == new_name - new_autoscale = not model.autoscale - model.autoscale = new_autoscale - assert view._auto_clim.value == new_autoscale - def test_JupyterLutView_update_view(model: LUTModel, view: JupyterLutView) -> None: """Ensures the model updates when the view is changed.""" - new_clims = (5, 6) - assert model.clims != new_clims - view._clims.value = new_clims - assert model.clims == new_clims - new_visible = not model.visible view._visible.value = new_visible assert view._visible.value == new_visible @@ -62,11 +53,12 @@ def test_JupyterLutView_update_view(model: LUTModel, view: JupyterLutView) -> No view._cmap.value = new_cmap assert model.cmap == new_cmap - new_autoscale = not model.autoscale - view._auto_clim.value = new_autoscale - assert model.autoscale == new_autoscale + mi, ma = view._clims.value + new_clims = ClimsManual(min=mi, max=ma) if view._auto_clim.value else ClimsMinMax() + view._auto_clim.value = not new_clims.is_manual + assert model.clims == new_clims # When gui clims change, autoscale should be disabled - model.autoscale = True + model.clims = ClimsMinMax() view._clims.value = (0, 1) - assert model.autoscale is False + assert model.clims == ClimsManual(min=0, max=1) # type:ignore diff --git a/tests/views/qt/test_lut_view.py b/tests/views/qt/test_lut_view.py index 5fa1ee40..d3f7cf8a 100644 --- a/tests/views/qt/test_lut_view.py +++ b/tests/views/qt/test_lut_view.py @@ -3,7 +3,7 @@ import cmap from pytest import fixture -from ndv.models._lut_model import LUTModel +from ndv.models._lut_model import ClimsManual, ClimsMinMax, LUTModel from ndv.views._app import QtProvider from ndv.views._qt._array_view import QLutView @@ -32,10 +32,10 @@ def view(model: LUTModel) -> QLutView: def test_QLutView_update_model(model: LUTModel, view: QLutView) -> None: """Ensures the view updates when the model is changed.""" - new_clims = (4, 5) - assert view._qwidget.clims.value() != new_clims - model.clims = new_clims - assert view._qwidget.clims.value() == new_clims + auto_scale = not model.clims.is_manual + assert view._qwidget.auto_clim.isChecked() == auto_scale + model.clims = ClimsManual(min=0, max=1) if auto_scale else ClimsMinMax() + assert view._qwidget.auto_clim.isChecked() != auto_scale new_visible = not model.visible model.visible = new_visible @@ -46,19 +46,10 @@ def test_QLutView_update_model(model: LUTModel, view: QLutView) -> None: model.cmap = new_cmap assert view._qwidget.cmap.currentColormap() == new_cmap - new_autoscale = not model.autoscale - model.autoscale = new_autoscale - assert view._qwidget.auto_clim.isChecked() == new_autoscale - def test_QLutView_update_view(model: LUTModel, view: QLutView) -> None: """Ensures the model updates when the view is changed.""" - new_clims = (5, 6) - assert model.clims != new_clims - view._qwidget.clims.setValue(new_clims) - assert model.clims == new_clims - new_visible = not model.visible view._qwidget.visible.setChecked(new_visible) assert view._qwidget.visible.isChecked() == new_visible @@ -69,11 +60,16 @@ def test_QLutView_update_view(model: LUTModel, view: QLutView) -> None: view._qwidget.cmap.setCurrentIndex(1) assert model.cmap == new_cmap - new_autoscale = not model.autoscale - view._qwidget.auto_clim.setChecked(new_autoscale) - assert model.autoscale == new_autoscale + mi, ma = view._qwidget.clims.value() + new_clims = ( + ClimsManual(min=mi, max=ma) + if view._qwidget.auto_clim.isChecked() + else ClimsMinMax() + ) + view._qwidget.auto_clim.setChecked(not new_clims.is_manual) + assert model.clims == new_clims # When gui clims change, autoscale should be disabled - model.autoscale = True + model.clims = ClimsMinMax() view._qwidget.clims.setValue((0, 1)) - assert model.autoscale is False + assert model.clims == ClimsManual(min=0, max=1) # type:ignore diff --git a/tests/views/wx_frontend/test_lut_view.py b/tests/views/wx_frontend/test_lut_view.py index fd1d3ba5..3b923ef8 100644 --- a/tests/views/wx_frontend/test_lut_view.py +++ b/tests/views/wx_frontend/test_lut_view.py @@ -4,7 +4,7 @@ import wx from pytest import fixture -from ndv.models._lut_model import LUTModel +from ndv.models._lut_model import ClimsManual, ClimsMinMax, LUTModel from ndv.views._app import WxProvider from ndv.views._wx._array_view import WxLutView @@ -35,10 +35,10 @@ def view(app: wx.App, model: LUTModel) -> WxLutView: def test_WxLutView_update_model(model: LUTModel, view: WxLutView) -> None: """Ensures the view updates when the model is changed.""" - new_clims = (4, 5) - assert view._wxwidget.clims.GetValues() != new_clims - model.clims = new_clims - assert view._wxwidget.clims.GetValues() == new_clims + auto_scale = not model.clims.is_manual + assert view._wxwidget.auto_clim.GetValue() == auto_scale + model.clims = ClimsManual(min=0, max=1) if auto_scale else ClimsMinMax() + assert view._wxwidget.auto_clim.GetValue != auto_scale new_visible = not model.visible model.visible = new_visible @@ -49,10 +49,6 @@ def test_WxLutView_update_model(model: LUTModel, view: WxLutView) -> None: model.cmap = new_cmap assert view._wxwidget.cmap.GetValue() == new_cmap - new_autoscale = not model.autoscale - model.autoscale = new_autoscale - assert view._wxwidget.auto_clim.GetValue() == new_autoscale - def test_WxLutView_update_view(app: wx.App, model: LUTModel, view: WxLutView) -> None: """Ensures the model updates when the view is changed.""" @@ -66,12 +62,12 @@ def processEvent(evt: wx.PyEventBinder, wdg: wx.Control) -> None: wx.EventLoopActivator(evtLoop) evtLoop.YieldFor(wx.EVT_CATEGORY_ALL) - new_clims = (5, 6) - assert model.clims != new_clims - clim_wdg = view._wxwidget.clims - clim_wdg.SetValue(*new_clims) - processEvent(wx.EVT_SLIDER, clim_wdg) - assert model.clims == new_clims + # new_clims = (5, 6) + # assert model.clims != new_clims + # clim_wdg = view._wxwidget.clims + # clim_wdg.SetValue(*new_clims) + # processEvent(wx.EVT_SLIDER, clim_wdg) + # assert model.clims == new_clims new_visible = not model.visible vis_wdg = view._wxwidget.visible @@ -86,14 +82,16 @@ def processEvent(evt: wx.PyEventBinder, wdg: wx.Control) -> None: processEvent(wx.EVT_COMBOBOX, cmap_wdg) assert model.cmap == new_cmap - new_autoscale = not model.autoscale + mi, ma = view._wxwidget.clims.GetValues() auto_wdg = view._wxwidget.auto_clim - auto_wdg.SetValue(new_autoscale) + new_clims = ClimsManual(min=mi, max=ma) if auto_wdg.GetValue() else ClimsMinMax() + view._wxwidget.auto_clim.SetValue(not new_clims.is_manual) processEvent(wx.EVT_TOGGLEBUTTON, auto_wdg) - assert model.autoscale == new_autoscale + assert model.clims == new_clims # When gui clims change, autoscale should be disabled - model.autoscale = True + model.clims = ClimsMinMax() + clim_wdg = view._wxwidget.clims clim_wdg.SetValue(0, 1) processEvent(wx.EVT_SLIDER, clim_wdg) - assert model.autoscale is False + assert model.clims == ClimsManual(min=0, max=1) # type:ignore From 795bfb9f1e08ffff061733e8fe0a0fcfd51fb380 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 17 Jan 2025 11:49:17 -0600 Subject: [PATCH 21/33] use qtbot to manage QLutView --- tests/views/qt/test_lut_view.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/views/qt/test_lut_view.py b/tests/views/qt/test_lut_view.py index d3f7cf8a..25cd8f03 100644 --- a/tests/views/qt/test_lut_view.py +++ b/tests/views/qt/test_lut_view.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import TYPE_CHECKING + import cmap from pytest import fixture @@ -7,6 +9,9 @@ from ndv.views._app import QtProvider from ndv.views._qt._array_view import QLutView +if TYPE_CHECKING: + from pytestqt.qtbot import QtBot + @fixture(autouse=True) def init_provider() -> None: @@ -20,8 +25,9 @@ def model() -> LUTModel: @fixture -def view(model: LUTModel) -> QLutView: +def view(model: LUTModel, qtbot: QtBot) -> QLutView: view = QLutView() + qtbot.add_widget(view.frontend_widget()) # Set the model assert view.model is None view.model = model From 855d00e28dc822a767ab26c014cfbf842b86d602 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 17 Jan 2025 13:33:47 -0600 Subject: [PATCH 22/33] Test controller autoscaling logic --- tests/test_controller.py | 41 +++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/tests/test_controller.py b/tests/test_controller.py index dd4a6240..832f2551 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -11,7 +11,7 @@ from ndv._types import MouseMoveEvent from ndv.controllers import ArrayViewer from ndv.models._array_display_model import ArrayDisplayModel, ChannelMode -from ndv.models._lut_model import LUTModel +from ndv.models._lut_model import ClimsManual, ClimsMinMax, LUTModel from ndv.views import _app, gui_frontend from ndv.views.bases import ArrayView, LutView from ndv.views.bases._graphics._canvas import ArrayCanvas, HistogramCanvas @@ -204,21 +204,24 @@ def test_array_viewer_with_app() -> None: assert viewer.display_model.visible_axes == (0, -2, -1) -# @pytest.mark.usefixtures("any_app") -# def test_channel_autoscale() -> None: -# data = np.random.randint(0, 255, size=(10, 10, 10, 10, 10), dtype="uint8") -# viewer = ArrayViewer(data) - -# lut_model = viewer._lut_controllers[None].lut_model -# old_clims = lut_model.clims - -# # Adjust the clims with autoscale off -# lut_model.autoscale = False -# lut_model.clims = (old_clims[0] + 1, old_clims[1] + 1) - -# # Assert turning autoscale back on reverts the clims -# lut_model.autoscale = True -# assert lut_model.clims == old_clims - -# # NB: The view is (currently) responsible for disabling autoscale when -# # it moves a clim. Thus that behavior is tested for each backend. +@pytest.mark.usefixtures("any_app") +def test_channel_autoscale() -> None: + # NB: Use a planar dataset so we can manually compute the min/max + data = np.random.randint(0, 255, size=(10, 10), dtype="uint8") + mi, ma = np.nanmin(data), np.nanmax(data) + viewer = ArrayViewer(data) + + # Test some random LutController + lut_ctrl = next(iter(viewer._lut_controllers.values())) + lut_model = lut_ctrl.lut_model + lut_model.clims = ClimsManual(min=1, max=2) + + # Ensure newly added lut views have the correct clims + mock_viewer = MagicMock(LutView) + lut_ctrl.add_lut_view(mock_viewer) + mock_viewer.set_clims.assert_called_once_with((1, 2)) + + # Ensure autoscaling sets the clims + mock_viewer.set_clims.reset_mock() + lut_model.clims = ClimsMinMax() + mock_viewer.set_clims.assert_called_once_with((mi, ma)) From a653af718031d0b338fa252ba3587f1ba3bdb38c Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 17 Jan 2025 13:48:07 -0600 Subject: [PATCH 23/33] Make test smaller The use of a controller was causing failures in some places. --- tests/test_controller.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/test_controller.py b/tests/test_controller.py index 832f2551..f57c9d98 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Callable, cast, no_type_check +from typing import Any, Callable, cast, no_type_check from unittest.mock import MagicMock, Mock, patch import numpy as np @@ -10,6 +10,7 @@ from ndv._types import MouseMoveEvent from ndv.controllers import ArrayViewer +from ndv.controllers._channel_controller import ChannelController from ndv.models._array_display_model import ArrayDisplayModel, ChannelMode from ndv.models._lut_model import ClimsManual, ClimsMinMax, LUTModel from ndv.views import _app, gui_frontend @@ -17,9 +18,6 @@ from ndv.views.bases._graphics._canvas import ArrayCanvas, HistogramCanvas from ndv.views.bases._graphics._canvas_elements import ImageHandle -if TYPE_CHECKING: - from ndv.controllers._channel_controller import ChannelController - def _get_mock_canvas() -> ArrayCanvas: mock = MagicMock(spec=ArrayCanvas) @@ -206,19 +204,22 @@ def test_array_viewer_with_app() -> None: @pytest.mark.usefixtures("any_app") def test_channel_autoscale() -> None: + ctrl = ChannelController(key=None, lut_model=LUTModel(), views=[]) + # NB: Use a planar dataset so we can manually compute the min/max data = np.random.randint(0, 255, size=(10, 10), dtype="uint8") mi, ma = np.nanmin(data), np.nanmax(data) - viewer = ArrayViewer(data) + handle = MagicMock(spec=ImageHandle) + handle.data.return_value = data + ctrl.add_handle(handle) # Test some random LutController - lut_ctrl = next(iter(viewer._lut_controllers.values())) - lut_model = lut_ctrl.lut_model + lut_model = ctrl.lut_model lut_model.clims = ClimsManual(min=1, max=2) # Ensure newly added lut views have the correct clims mock_viewer = MagicMock(LutView) - lut_ctrl.add_lut_view(mock_viewer) + ctrl.add_lut_view(mock_viewer) mock_viewer.set_clims.assert_called_once_with((1, 2)) # Ensure autoscaling sets the clims From c3ce77acf89739637f03849c2f16b1e5e0cf18fe Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 15:15:18 -0600 Subject: [PATCH 24/33] Ad __eq__ for ClimPolicy impls Will prevent a fair number of signal reemissions. Thanks to @tlambert03 for the suggestion --- src/ndv/models/_lut_model.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/ndv/models/_lut_model.py b/src/ndv/models/_lut_model.py index 53a2cb16..377d2d87 100644 --- a/src/ndv/models/_lut_model.py +++ b/src/ndv/models/_lut_model.py @@ -64,6 +64,13 @@ class ClimsManual(ClimPolicy): def get_limits(self, data: npt.NDArray) -> tuple[float, float]: return self.min, self.max + def __eq__(self, other: object) -> bool: + return ( + isinstance(other, ClimsManual) + and self.min == other.min + and self.max == other.max + ) + class ClimsMinMax(ClimPolicy): """Autoscale contrast limits based on the minimum and maximum values in the data.""" @@ -73,6 +80,9 @@ class ClimsMinMax(ClimPolicy): def get_limits(self, data: npt.NDArray) -> tuple[float, float]: return (np.nanmin(data), np.nanmax(data)) + def __eq__(self, other: object) -> bool: + return isinstance(other, ClimsMinMax) + class ClimsPercentile(ClimPolicy): """Autoscale contrast limits based on percentiles of the data. @@ -92,6 +102,13 @@ class ClimsPercentile(ClimPolicy): def get_limits(self, data: npt.NDArray) -> tuple[float, float]: return tuple(np.nanpercentile(data, [self.min_percentile, self.max_percentile])) + def __eq__(self, other: object) -> bool: + return ( + isinstance(other, ClimsPercentile) + and self.min_percentile == other.min_percentile + and self.max_percentile == other.max_percentile + ) + class ClimsStdDev(ClimPolicy): """Automatically set contrast limits based on standard deviations from the mean. @@ -114,6 +131,13 @@ def get_limits(self, data: npt.NDArray) -> tuple[float, float]: diff = self.n_stdev * np.nanstd(data) return center - diff, center + diff + def __eq__(self, other: object) -> bool: + return ( + isinstance(other, ClimsStdDev) + and self.n_stdev == other.n_stdev + and self.center == other.center + ) + # we can add this, but it needs to have a proper pydantic serialization method # similar to ReducerType From aa7ac4743b20aedef65116c9d490615775a5d5fb Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 15:59:48 -0600 Subject: [PATCH 25/33] Clean up jupyter lutview Thanks again @tlambert03 for the suggestion --- src/ndv/views/_jupyter/_array_view.py | 41 +++++++++++++++------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/ndv/views/_jupyter/_array_view.py b/src/ndv/views/_jupyter/_array_view.py index 69486ba6..02489c3f 100644 --- a/src/ndv/views/_jupyter/_array_view.py +++ b/src/ndv/views/_jupyter/_array_view.py @@ -1,5 +1,6 @@ from __future__ import annotations +import contextlib import warnings from typing import TYPE_CHECKING, Any, cast @@ -10,9 +11,10 @@ from ndv.views.bases import ArrayView, LutView if TYPE_CHECKING: - from collections.abc import Container, Hashable, Mapping, Sequence + from collections.abc import Container, Hashable, Iterator, Mapping, Sequence import cmap + from traitlets import HasTraits from vispy.app.backends import _jupyter_rfb from ndv._types import AxisKey @@ -22,6 +24,19 @@ # i think it has to do with type variance? +@contextlib.contextmanager +def notifications_blocked( + obj: HasTraits, name: str = "value", type: str = "change" +) -> Iterator[None]: + # traitlets doesn't provide a public API for this + notifiers: list | None = obj._trait_notifiers.get(name, {}).pop(type, None) + try: + yield + finally: + if notifiers is not None: + obj._trait_notifiers[name][type] = notifiers + + class JupyterLutView(LutView): def __init__(self) -> None: # WIDGETS @@ -71,25 +86,24 @@ def __init__(self) -> None: self._cmap.observe(self._on_cmap_changed, names="value") self._clims.observe(self._on_clims_changed, names="value") self._auto_clim.observe(self._on_autoscale_changed, names="value") - self._updating: bool = False # ------------------ emit changes to the controller ------------------ def _on_clims_changed(self, change: dict[str, Any]) -> None: - if self._model and not self._updating: + if self._model: clims = self._clims.value self._model.clims = ClimsManual(min=clims[0], max=clims[1]) def _on_visible_changed(self, change: dict[str, Any]) -> None: - if self._model and not self._updating: + if self._model: self._model.visible = self._visible.value def _on_cmap_changed(self, change: dict[str, Any]) -> None: - if self._model and not self._updating: + if self._model: self._model.cmap = self._cmap.value def _on_autoscale_changed(self, change: dict[str, Any]) -> None: - if self._model and not self._updating: + if self._model: if change["new"]: # Autoscale self._model.clims = ClimsMinMax() else: # Manually scale @@ -101,28 +115,19 @@ def _on_autoscale_changed(self, change: dict[str, Any]) -> None: def set_channel_name(self, name: str) -> None: self._visible.description = name - # NOTE: it's important to block signals when setting values from the controller - # to avoid loops, unnecessary updates, and unexpected behavior - def set_clim_policy(self, policy: ClimPolicy) -> None: - self._updating = True self._auto_clim.value = not policy.is_manual - self._updating = False def set_colormap(self, cmap: cmap.Colormap) -> None: - self._updating = True self._cmap.value = cmap.name.split(":")[-1] # FIXME: this is a hack - self._updating = False def set_clims(self, clims: tuple[float, float]) -> None: - self._updating = True - self._clims.value = clims - self._updating = False + # block self._clims.observe, otherwise autoscale will be forced off + with notifications_blocked(self._clims): + self._clims.value = clims def set_channel_visible(self, visible: bool) -> None: - self._updating = True self._visible.value = visible - self._updating = False def set_gamma(self, gamma: float) -> None: pass From 2964408fb94941b5b07a385182b42e27a2f5d312 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 16:05:25 -0600 Subject: [PATCH 26/33] Clean up qt lutview --- src/ndv/views/_qt/_array_view.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index ece719d6..973a6dbf 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -154,12 +154,10 @@ def set_channel_name(self, name: str) -> None: self._qwidget.visible.setText(name) def set_clim_policy(self, policy: ClimPolicy) -> None: - with signals_blocked(self._qwidget.auto_clim): - self._qwidget.auto_clim.setChecked(not policy.is_manual) + self._qwidget.auto_clim.setChecked(not policy.is_manual) def set_colormap(self, cmap: cmap.Colormap) -> None: - with signals_blocked(self._qwidget.cmap): - self._qwidget.cmap.setCurrentColormap(cmap) + self._qwidget.cmap.setCurrentColormap(cmap) def set_clims(self, clims: tuple[float, float]) -> None: with signals_blocked(self._qwidget.clims): From c423d09bac2cc7e41551c0c4cd75507268dbe64a Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 16:16:11 -0600 Subject: [PATCH 27/33] LutView: Strengthen method descriptions --- src/ndv/views/bases/_lut_view.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ndv/views/bases/_lut_view.py b/src/ndv/views/bases/_lut_view.py index b5668353..64cb356c 100644 --- a/src/ndv/views/bases/_lut_view.py +++ b/src/ndv/views/bases/_lut_view.py @@ -24,13 +24,20 @@ def set_channel_name(self, name: str) -> None: @abstractmethod def set_clim_policy(self, policy: ClimPolicy) -> None: - """Set the autoscale button to checked if `checked` is True.""" + """Set the clim policy to `policy`. + + Usually corresponds to an "autoscale" checkbox. + + Note that this method must not modify the backing LUTModel. + """ @abstractmethod def set_colormap(self, cmap: cmap.Colormap) -> None: """Set the colormap to `cmap`. Usually corresponds to a dropdown menu. + + Note that this method must not modify the backing LUTModel. """ @abstractmethod @@ -38,6 +45,8 @@ def set_clims(self, clims: tuple[float, float]) -> None: """Set the (low, high) contrast limits to `clims`. Usually this will be a range slider or two text boxes. + + Note that this method must not modify the backing LUTModel. """ @abstractmethod @@ -45,10 +54,15 @@ def set_channel_visible(self, visible: bool) -> None: """Check or uncheck the visibility indicator of the LUT. Usually corresponds to a checkbox. + + Note that this method must not modify the backing LUTModel. """ def set_gamma(self, gamma: float) -> None: - """Set the gamma value of the LUT.""" + """Set the gamma value of the LUT. + + Note that this method must not modify the backing LUTModel. + """ return None @property From 22d2de227e93452b91cd4f14934d5956cc25fbca Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 16:17:25 -0600 Subject: [PATCH 28/33] vispy: remove fps printout --- src/ndv/views/_vispy/_array_canvas.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ndv/views/_vispy/_array_canvas.py b/src/ndv/views/_vispy/_array_canvas.py index be300cbd..d6e8d2f2 100755 --- a/src/ndv/views/_vispy/_array_canvas.py +++ b/src/ndv/views/_vispy/_array_canvas.py @@ -426,7 +426,6 @@ class VispyArrayCanvas(ArrayCanvas): def __init__(self) -> None: self._canvas = scene.SceneCanvas(size=(600, 600)) - self._canvas.measure_fps() # this filter needs to remain in scope for the lifetime of the canvas # or mouse events will not be intercepted From 485d41ebf57e31f0d616a938c6578e05ff4e7871 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 17:51:26 -0600 Subject: [PATCH 29/33] Cleanup --- src/ndv/views/_pygfx/_array_canvas.py | 2 -- src/ndv/views/bases/_graphics/_canvas_elements.py | 4 ---- src/ndv/views/bases/_lut_view.py | 2 -- tests/views/wx_frontend/test_lut_view.py | 12 ++++++------ 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/ndv/views/_pygfx/_array_canvas.py b/src/ndv/views/_pygfx/_array_canvas.py index ec7597e4..f09fbeb2 100755 --- a/src/ndv/views/_pygfx/_array_canvas.py +++ b/src/ndv/views/_pygfx/_array_canvas.py @@ -76,8 +76,6 @@ def gamma(self) -> float: return 1 def set_gamma(self, gamma: float) -> None: - # self._material.gamma = gamma - # self._render() if gamma != 1: warnings.warn("Gamma correction is not supported in pygfx", stacklevel=2) diff --git a/src/ndv/views/bases/_graphics/_canvas_elements.py b/src/ndv/views/bases/_graphics/_canvas_elements.py index ebeac28b..a12eee22 100644 --- a/src/ndv/views/bases/_graphics/_canvas_elements.py +++ b/src/ndv/views/bases/_graphics/_canvas_elements.py @@ -94,10 +94,6 @@ def set_channel_name(self, name: str) -> None: def set_clim_policy(self, policy: ClimPolicy) -> None: pass - # if checked and self.model: - # d = self.data() - # self.model.clims = (d.min(), d.max()) - def set_channel_visible(self, visible: bool) -> None: self.set_visible(visible) diff --git a/src/ndv/views/bases/_lut_view.py b/src/ndv/views/bases/_lut_view.py index 64cb356c..99d3be58 100644 --- a/src/ndv/views/bases/_lut_view.py +++ b/src/ndv/views/bases/_lut_view.py @@ -10,8 +10,6 @@ from ndv.models._lut_model import ClimPolicy, LUTModel - pass - class LutView(Viewable): """Manages LUT properties (contrast, colormap, etc...) in a view object.""" diff --git a/tests/views/wx_frontend/test_lut_view.py b/tests/views/wx_frontend/test_lut_view.py index 5f30465d..fb97f7ea 100644 --- a/tests/views/wx_frontend/test_lut_view.py +++ b/tests/views/wx_frontend/test_lut_view.py @@ -62,12 +62,12 @@ def processEvent(evt: wx.PyEventBinder, wdg: wx.Control) -> None: wx.EventLoopActivator(evtLoop) evtLoop.YieldFor(wx.EVT_CATEGORY_ALL) - # new_clims = (5, 6) - # assert model.clims != new_clims - # clim_wdg = view._wxwidget.clims - # clim_wdg.SetValue(*new_clims) - # processEvent(wx.EVT_SLIDER, clim_wdg) - # assert model.clims == new_clims + new_clims = (5, 6) + assert model.clims != new_clims + clim_wdg = view._wxwidget.clims + clim_wdg.SetValue(*new_clims) + processEvent(wx.EVT_SLIDER, clim_wdg) + assert model.clims == ClimsManual(min=5, max=6) new_visible = not model.visible vis_wdg = view._wxwidget.visible From 4bf81b500154f49ab89124e75e28105e7df44a95 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 18:01:19 -0600 Subject: [PATCH 30/33] Align src and test package names --- tests/views/{jupyter => _jupyter}/__init__.py | 0 tests/views/{jupyter => _jupyter}/test_lut_view.py | 0 tests/views/{qt => _qt}/__init__.py | 0 tests/views/{qt => _qt}/test_lut_view.py | 0 tests/views/{wx_frontend => _wx}/__init__.py | 0 tests/views/{wx_frontend => _wx}/test_lut_view.py | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename tests/views/{jupyter => _jupyter}/__init__.py (100%) rename tests/views/{jupyter => _jupyter}/test_lut_view.py (100%) rename tests/views/{qt => _qt}/__init__.py (100%) rename tests/views/{qt => _qt}/test_lut_view.py (100%) rename tests/views/{wx_frontend => _wx}/__init__.py (100%) rename tests/views/{wx_frontend => _wx}/test_lut_view.py (100%) diff --git a/tests/views/jupyter/__init__.py b/tests/views/_jupyter/__init__.py similarity index 100% rename from tests/views/jupyter/__init__.py rename to tests/views/_jupyter/__init__.py diff --git a/tests/views/jupyter/test_lut_view.py b/tests/views/_jupyter/test_lut_view.py similarity index 100% rename from tests/views/jupyter/test_lut_view.py rename to tests/views/_jupyter/test_lut_view.py diff --git a/tests/views/qt/__init__.py b/tests/views/_qt/__init__.py similarity index 100% rename from tests/views/qt/__init__.py rename to tests/views/_qt/__init__.py diff --git a/tests/views/qt/test_lut_view.py b/tests/views/_qt/test_lut_view.py similarity index 100% rename from tests/views/qt/test_lut_view.py rename to tests/views/_qt/test_lut_view.py diff --git a/tests/views/wx_frontend/__init__.py b/tests/views/_wx/__init__.py similarity index 100% rename from tests/views/wx_frontend/__init__.py rename to tests/views/_wx/__init__.py diff --git a/tests/views/wx_frontend/test_lut_view.py b/tests/views/_wx/test_lut_view.py similarity index 100% rename from tests/views/wx_frontend/test_lut_view.py rename to tests/views/_wx/test_lut_view.py From 4a1fc31141920d580bab3d02c345ce799686a484 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 18:08:17 -0600 Subject: [PATCH 31/33] Add note for QLutView.set_clims --- src/ndv/views/_qt/_array_view.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index b780feaa..eb613de4 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -183,6 +183,7 @@ def set_colormap(self, cmap: cmap.Colormap) -> None: self._qwidget.cmap.setCurrentColormap(cmap) def set_clims(self, clims: tuple[float, float]) -> None: + # block self._qwidget._clims, otherwise autoscale will be forced off with signals_blocked(self._qwidget.clims): if not isinstance(clims, tuple): breakpoint() From f9d20dd4c02adf58c8cbbff5e56b596e1720d0e0 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 18:12:53 -0600 Subject: [PATCH 32/33] Remove breakpoint() --- src/ndv/views/_qt/_array_view.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ndv/views/_qt/_array_view.py b/src/ndv/views/_qt/_array_view.py index eb613de4..78ae0f53 100644 --- a/src/ndv/views/_qt/_array_view.py +++ b/src/ndv/views/_qt/_array_view.py @@ -185,8 +185,6 @@ def set_colormap(self, cmap: cmap.Colormap) -> None: def set_clims(self, clims: tuple[float, float]) -> None: # block self._qwidget._clims, otherwise autoscale will be forced off with signals_blocked(self._qwidget.clims): - if not isinstance(clims, tuple): - breakpoint() self._qwidget.clims.setValue(clims) def set_gamma(self, gamma: float) -> None: From 7d9f4b24d6454f927887e9e8a2bee7d09f4af97f Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 4 Feb 2025 18:20:54 -0600 Subject: [PATCH 33/33] Remove unused ChannelController.clims --- src/ndv/controllers/_channel_controller.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ndv/controllers/_channel_controller.py b/src/ndv/controllers/_channel_controller.py index 3e800a75..22c1a171 100644 --- a/src/ndv/controllers/_channel_controller.py +++ b/src/ndv/controllers/_channel_controller.py @@ -32,7 +32,6 @@ def __init__( self.lut_model = lut_model self.lut_model.events.clims.connect(self._auto_scale) self.handles: list[ImageHandle] = [] - self.clims: tuple[float, float] = (0, 1) for v in views: self.add_lut_view(v)