Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Pass LUTModel to LUTView #87

Merged
merged 39 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
0fbe95a
Convert LUTView to Model-View
gselzer Jan 12, 2025
cc931db
rename views
tlambert03 Jan 12, 2025
8d0ffcf
Merge branch 'main' into lut-mv
tlambert03 Jan 12, 2025
1a4107d
Remove set_colormap_without_signal usage in tests
gselzer Jan 13, 2025
9abec15
Sync autoscale and clims
gselzer Jan 13, 2025
e3e1f27
Merge remote-tracking branch 'upstream/main' into lut-mv
gselzer Jan 13, 2025
f64bb06
ImageHandle.cmap -> ImageHandle.colormap
gselzer Jan 13, 2025
ae478c3
PyGFXImageHandle.set_gamma: Allow gamma=1
gselzer Jan 13, 2025
6abb551
Clean up LutView
gselzer Jan 13, 2025
be5b581
Comment _channel_controller.py
gselzer Jan 13, 2025
14eb731
More cleanups
gselzer Jan 13, 2025
1f7c957
Add tests for QLutView
gselzer Jan 13, 2025
8f82dd0
Fix qt check
gselzer Jan 13, 2025
f9c04ed
Fix pytest qt skip
gselzer Jan 13, 2025
53ef36a
Add wx to dev extra
gselzer Jan 14, 2025
24fbc45
Add LutView tests for Jupyter, Wx
gselzer Jan 14, 2025
617f77c
Improve wx tests
gselzer Jan 14, 2025
60ac456
Move autoscale clims to controller
gselzer Jan 14, 2025
16237f2
Fix _CmapCombo.setCurrentColormap
gselzer Jan 14, 2025
32b578a
Test enabling autoscaling
gselzer Jan 14, 2025
3536f44
Merge remote-tracking branch 'upstream/main' into lut-mv
gselzer Jan 17, 2025
409c89b
Fixes
gselzer Jan 17, 2025
c3498fd
Fix tests
gselzer Jan 17, 2025
aa27b69
Merge remote-tracking branch 'upstream/main' into lut-mv
gselzer Jan 17, 2025
795bfb9
use qtbot to manage QLutView
gselzer Jan 17, 2025
855d00e
Test controller autoscaling logic
gselzer Jan 17, 2025
a653af7
Make test smaller
gselzer Jan 17, 2025
c3ce77a
Ad __eq__ for ClimPolicy impls
gselzer Feb 4, 2025
aa7ac47
Clean up jupyter lutview
gselzer Feb 4, 2025
2964408
Clean up qt lutview
gselzer Feb 4, 2025
c423d09
LutView: Strengthen method descriptions
gselzer Feb 4, 2025
22d2de2
vispy: remove fps printout
gselzer Feb 4, 2025
d770560
Merge remote-tracking branch 'upstream/main' into lut-mv
gselzer Feb 4, 2025
266b94a
Merge branch 'main' into lut-mv
tlambert03 Feb 4, 2025
485d41e
Cleanup
gselzer Feb 4, 2025
4bf81b5
Align src and test package names
gselzer Feb 5, 2025
4a1fc31
Add note for QLutView.set_clims
gselzer Feb 5, 2025
f9d20dd
Remove breakpoint()
gselzer Feb 5, 2025
7d9f4b2
Remove unused ChannelController.clims
gselzer Feb 5, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/notebook.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.7"
"version": "3.12.0"
}
},
"nbformat": 4,
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ docs = [
"ruff",
]
dev = [
"ndv[test,vispy,pygfx,pyqt,jupyter]",
"ndv[test,vispy,pygfx,pyqt,jupyter,wxpython]",
"pytest-qt",
"ipython",
"mypy",
Expand Down
10 changes: 5 additions & 5 deletions src/ndv/controllers/_array_viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,10 @@
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:

Check warning on line 184 in src/ndv/controllers/_array_viewer.py

View check run for this annotation

Codecov / codecov/patch

src/ndv/controllers/_array_viewer.py#L184

Added line #L184 was not covered by tests
data = handles[0].data()
counts, edges = _calc_hist_bins(data)
self._histogram.set_data(counts, edges)
Expand Down Expand Up @@ -246,7 +246,7 @@
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:
Expand Down Expand Up @@ -356,7 +356,7 @@
lut_views.append(self._histogram)
self._lut_controllers[key] = lut_ctrl = ChannelController(
key=key,
model=model,
lut_model=model,
views=lut_views,
)

Expand Down
134 changes: 28 additions & 106 deletions src/ndv/controllers/_channel_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@
from contextlib import suppress
from typing import TYPE_CHECKING

from ndv.models._lut_model import ClimsManual, ClimsMinMax, ClimsType

if TYPE_CHECKING:
from collections.abc import Iterable, Sequence

import cmap
import numpy as np

from ndv.models._lut_model import LUTModel
Expand All @@ -27,113 +24,34 @@
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.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)

# 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.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: ClimsType) -> None:
"""The contrast limits in the model have changed."""
is_autoscale = not clims.is_manual
for handle in self.handles:
min_max = clims.calc_clims(handle.data())
handle.set_clims(min_max)
for v in self.lut_views:
v.set_clims_without_signal(min_max)
v.set_auto_scale_without_signal(is_autoscale)

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)

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:
"""Make sure the view matches the model."""
# TODO: Could probably reuse cached clims
self._auto_scale()

def synchronize(self, *views: LutView) -> None:
"""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:
view.set_colormap_without_signal(self.lut_model.cmap)
if self.lut_model.clims and (clims := self.lut_model.clims.cached_clims):
view.set_clims_without_signal(clims)

is_autoscale = not self.lut_model.clims.is_manual
view.set_auto_scale_without_signal(is_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:
previous = handle.visible()
handle.set_visible(visible)
if previous != visible:
self._update_clims(handle)

def _on_view_lut_autoscale_changed(
self, autoscale: bool, key: LutKey = None
) -> None:
"""The autoscale checkbox in the LUT widget has changed."""
if autoscale:
self.lut_model.clims = ClimsMinMax()
elif cached := self.lut_model.clims.cached_clims:
self.lut_model.clims = ClimsManual(min=cached[0], max=cached[1])

for view in self.lut_views:
view.set_auto_scale_without_signal(autoscale)

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 = ClimsManual(min=clims[0], max=clims[1])

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:
Expand All @@ -143,20 +61,12 @@
return
handle = handles[0]
handle.set_data(data)
if handle.visible():
self._update_clims(handle)
self._auto_scale()

def add_handle(self, handle: ImageHandle) -> None:
"""Add an image texture handle to the controller."""
self.handles.append(handle)
handle.set_cmap(self.lut_model.cmap)
self._update_clims(handle)

def _update_clims(self, handle: ImageHandle) -> None:
min_max = self.lut_model.clims.calc_clims(handle.data())
handle.set_clims(min_max)
for view in self.lut_views:
view.set_clims_without_signal(min_max)
self.add_lut_view(handle)

def get_value_at_index(self, idx: tuple[int, ...]) -> float | None:
"""Get the value of the data at the given index."""
Expand All @@ -174,3 +84,15 @@
# the data source directly.
return handle.data()[idx] # type: ignore [no-any-return]
return None

def _auto_scale(self) -> None:
if self.lut_model and len(self.handles):
policy = self.lut_model.clims
handle_clims = [policy.calc_clims(handle.data()) for handle in self.handles]
mi, ma = handle_clims[0]
for clims in handle_clims[1:]:
mi = min(mi, clims[0])
ma = max(ma, clims[1])

Check warning on line 95 in src/ndv/controllers/_channel_controller.py

View check run for this annotation

Codecov / codecov/patch

src/ndv/controllers/_channel_controller.py#L94-L95

Added lines #L94 - L95 were not covered by tests

for view in self.lut_views:
view.set_clims((mi, ma))
4 changes: 2 additions & 2 deletions src/ndv/v1/_old_viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/ndv/v1/_qt/_lut_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
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)

Expand Down Expand Up @@ -110,7 +110,7 @@

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 (
Expand Down Expand Up @@ -139,5 +139,5 @@

def add_handle(self, handle: ImageHandle) -> None:
self._handles.append(handle)
self._cmap.addColormap(handle.cmap())
self._cmap.addColormap(handle.colormap())

Check warning on line 142 in src/ndv/v1/_qt/_lut_control.py

View check run for this annotation

Codecov / codecov/patch

src/ndv/v1/_qt/_lut_control.py#L142

Added line #L142 was not covered by tests
self.update_autoscale()
43 changes: 29 additions & 14 deletions src/ndv/views/_jupyter/_array_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@
import warnings
from typing import TYPE_CHECKING, Any, cast

import cmap
import ipywidgets as widgets

from ndv.models._array_display_model import ChannelMode
from ndv.models._lut_model import ClimPolicy, ClimsManual, ClimsMinMax
from ndv.views.bases import ArrayView, LutView

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
Expand Down Expand Up @@ -70,20 +71,30 @@
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:
self.climsChanged.emit(self._clims.value)
if self._model and not self._updating:
clims = self._clims.value
self._model.clims = ClimsManual(min=clims[0], max=clims[1])

def _on_visible_changed(self, change: dict[str, Any]) -> None:
self.visibilityChanged.emit(self._visible.value)
if self._model and not self._updating:
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 and not self._updating:
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 and not self._updating:
if change["new"]: # Autoscale
self._model.clims = ClimsMinMax()
else: # Manually scale

Check warning on line 95 in src/ndv/views/_jupyter/_array_view.py

View check run for this annotation

Codecov / codecov/patch

src/ndv/views/_jupyter/_array_view.py#L95

Added line #L95 was not covered by tests
clims = self._clims.value
self._model.clims = ClimsManual(min=clims[0], max=clims[1])

# ------------------ receive changes from the controller ---------------

Expand All @@ -93,21 +104,25 @@
# 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:
with self.autoscaleChanged.blocked():
self._auto_clim.value = auto
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:
with self.cmapChanged.blocked():
self._cmap.value = cmap.name.split(":")[-1] # FIXME: this is a hack
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._clims.value = clims
self._updating = True
self._clims.value = clims
self._updating = False

def set_channel_visible(self, visible: bool) -> None:
with self.visibilityChanged.blocked():
self._visible.value = visible
self._updating = True
self._visible.value = visible
self._updating = False

def set_gamma(self, gamma: float) -> None:
pass
Expand Down
7 changes: 4 additions & 3 deletions src/ndv/views/_pygfx/_array_canvas.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@
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)

Check warning on line 82 in src/ndv/views/_pygfx/_array_canvas.py

View check run for this annotation

Codecov / codecov/patch

src/ndv/views/_pygfx/_array_canvas.py#L82

Added line #L82 was not covered by tests

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()
Expand Down
Loading
Loading