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

feat: V2 add back channel modes and multi-channel display #57

Merged
merged 9 commits into from
Dec 8, 2024

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Nov 25, 2024

This pull request includes several updates and improvements to the project, focusing on dependency updates, documentation enhancements, and significant changes to the ViewerController class in the src/ndv/controller/_controller.py file. Here are the most important changes:

Dependency Updates:

  • Updated versions of dependencies in .pre-commit-config.yaml:
    • validate-pyproject updated from v0.22 to v0.23.
    • typos updated from v1.27.0 to typos-dict-v0.11.35.
    • ruff-pre-commit updated from v0.7.2 to v0.8.0.

Documentation Enhancements:

  • Added installation instructions for Qt and Jupyter in README.md.

ViewerController Class Updates:

  • Added ChannelMode and LUTModel imports and refactored ViewerController to utilize these models.
  • Introduced channelModeChanged signal connection and _on_model_channel_mode_changed method to handle channel mode changes. [1] [2]
  • Removed and refactored methods related to LUT visibility, autoscale, and colormap changes, consolidating them into the channel_mode handling.
  • Updated _update_canvas method to handle asynchronous data requests and manage LUT controllers.

Miscellaneous Changes:

  • Added optional dependencies for qt and jupyter in pyproject.toml.
  • Corrected module override for mypy from ndv._old_viewer.* to ndv.old_viewer.* in pyproject.toml.

@tlambert03 tlambert03 changed the base branch from main to v2-mvc November 25, 2024 03:50
@tlambert03
Copy link
Member Author

plenty of work left to be done, but the dual qt/jupyter dream is evolving! In general, I'm happy with how easy it is to implement two different front ends, with most of the logic being in the controller..

Untitled.mov

cc @fdrgsp, @gselzer 👀

Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 64.78405% with 106 lines in your changes missing coverage. Please review.

Please upload report for BASE (v2-mvc@39ca37d). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/ndv/controller/_controller.py 57.85% 51 Missing ⚠️
src/ndv/views/_jupyter/jupyter_view.py 0.00% 31 Missing ⚠️
src/ndv/models/_data_display_model.py 78.57% 12 Missing ⚠️
src/ndv/views/_vispy/_vispy.py 58.82% 7 Missing ⚠️
src/ndv/views/protocols.py 85.71% 2 Missing ⚠️
src/ndv/old_viewer/_old_viewer.py 0.00% 1 Missing ⚠️
src/ndv/views/_pygfx/_pygfx.py 50.00% 1 Missing ⚠️
src/ndv/views/_qt/qt_view.py 97.36% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             v2-mvc      #57   +/-   ##
=========================================
  Coverage          ?   74.59%           
=========================================
  Files             ?       28           
  Lines             ?     2925           
  Branches          ?        0           
=========================================
  Hits              ?     2182           
  Misses            ?      743           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tlambert03
Copy link
Member Author

hey @gselzer,
this PR is starting to feel pretty good, and probably mergable soon. Would be grateful for your input if you have time to play with it. Mostly, the goal here was to get channel modes working again, so that the appropriate sliders hide/show and gray/composite modes work again. Color mode and RGB are still not working yet

Copy link
Collaborator

@gselzer gselzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is super awesome! One major step forwards towards parity with the old viewer!

I had a couple suggestions, but more importantly it seems like RGB mode is broken right now? When I run the following script, I see a 256-channel image, and switching to RGB mode crashes the viewer:

from qtpy.QtWidgets import QApplication

from skimage.data import astronaut
from ndv.controller import ViewerController
app = QApplication([])

viewer = ViewerController()  # ultimately, this will be the public api
_data = astronaut()
viewer.data = _data
viewer.show()
app.exec()

src/ndv/controller/_controller.py Outdated Show resolved Hide resolved
src/ndv/controller/_controller.py Outdated Show resolved Hide resolved
Comment on lines 243 to 236
handle = self._canvas.add_image(
data, cmap=lut_ctrl.lut_model.cmap, clims=lut_ctrl.lut_model.clims
)
lut_ctrl.handles.append(handle)
self._canvas.set_range()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a way, a PImageHandle could be considered a PLutView, no? I'd think we could simplify the code if we refactored the LutController to, instead of having a list[PImageHandle], to just have a list[PLutView]. But you/I could wait to try that in a separate PR...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a way, a PImageHandle could be considered a PLutView, no?

hadn't thought about it that way (yet). PImageHandle is something that would directly control a texture object in the canvas (vispy/pygfx) backend (it does not receive input), and PLutView is a widget that does receive input from the user to update the model. If the LutController doesn't have a set of handles to update directly, then we would need to have all of those connections between on_model_lut_changed and the thing that updates the vispy texture back on the main view object. So, at the moment, I think i'm happier having the controller be the one thing that negotiates changes to model/widget/texture for a given LUT. and will likely remove ImgHandles from th main view

src/ndv/controller/_controller.py Show resolved Hide resolved
src/ndv/data.py Show resolved Hide resolved
src/ndv/controller/_controller.py Show resolved Hide resolved
src/ndv/models/_data_display_model.py Show resolved Hide resolved
src/ndv/views/_vispy/_vispy.py Show resolved Hide resolved
@tlambert03
Copy link
Member Author

it seems like RGB mode is broken right now

yep, haven't yet worked on RGB mode.

@tlambert03
Copy link
Member Author

I'm going to revert this to a6e120e, before i started working on Color mode... and where I liked the state of it. Will merge and pick up different modes later

@tlambert03
Copy link
Member Author

merging so we can play with it in #55

@tlambert03 tlambert03 merged commit 1cf2ff2 into pyapp-kit:v2-mvc Dec 8, 2024
13 of 14 checks passed
@tlambert03 tlambert03 deleted the v2-add-channels branch December 8, 2024 17:42
@tlambert03 tlambert03 changed the title V2 add back channel modes and multi-channel display feat: V2 add back channel modes and multi-channel display Jan 17, 2025
@tlambert03 tlambert03 added the enhancement New feature or request label Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants