Highlight objects

[zoccoler]

This pull request introduces new functionality to focus the viewer on highlighted objects in the napari_clusters_plotter widget, along with utility functions for affine transformations and camera adjustments. The most important changes include adding a signal handler for object highlighting, implementing the logic for focusing on selected objects, and creating reusable affine transformation and camera utilities.

New functionality for object highlighting and focusing:

• Added a connection for the highlighted_changed_signal to the _on_highlighted_changed method in _setup_callbacks. This enables the widget to respond when an object is highlighted. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR186-R189)
• Implemented _on_highlighted_changed, which focuses the viewer on a highlighted object in the layer. It handles different layer types such as Points, Labels, Surface, Shapes, and Tracks. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR752-R948)

Utility functions for affine transformations:

• Added _apply_affine_transform to apply affine transformations to coordinates. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR752-R948)
• Added _build_affine_matrix to construct an affine transformation matrix from rotation, scaling, shearing, and translation components. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR752-R948)

Camera utilities for viewer adjustments:

• Added _set_viewer_camera to adjust the viewer's camera to focus on specified coordinates, including calculating an appropriate zoom level. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR752-R948)
• Added _calculate_default_zoom to compute the default zoom level for the viewer based on the scene size and margin. (src/napari_clusters_plotter/_new_plotter_widget.py, src/napari_clusters_plotter/_new_plotter_widget.pyR752-R948)

To do:

• [x] evaluate if current behavior (centering and zooming in) feels like a good ux
• [x] implement unhighlighting (when pressing Escape) condition (reset_view)?
• [x] write tests

[zoccoler]

This PR seems to be somewhat related to #441 . Here we highlight a point in the plotter and get a focusing effect in the corresponding object in the napari layer. There, if I understood it correctly, it is the other way around ?

[jo-mueller]

This PR seems to be somewhat related to https://github.com/BiAPoL/napari-clusters-plotter/pull/441 . Here we highlight a point in the plotter and get a focusing effect in the corresponding object in the napari layer. There, if I understood it correctly, it is the other way around ?

Correct! Full bi-directionality :)

As for this PR, I think the new way of calculating the Affine transform is correct, but there seem to be some conflicts with the main branch? Also, side-note: I think I would not change the zoom upon jumping to a new object. The zoom could be something that a user set specifically for their dataset. In that case I could imagine it to be a bit annoying if the zoom jumped to something else upon highlighting a datapoint. It's easily something that we can build in later (or just comment out at this time).

Orrr we both try it and see how it feels ona few datasets. ^^"

[zoccoler]

Alright, so I commented the zoom part for now, added tests for the points layer, solved conflicts, and fixed the pre-commit complaints.

An unhighlighting feature is not needed if we don't zoom in IMO, but we may bring it in at another PR.

Tests were passing locally, not sure why they are failing now. I can check here again next week as I will be out for a couple days. @jo-mueller feel free to jump in if you find the need to have this merged before that.

[jo-mueller]

@zoccoler I think I see what's happening - in some recent change, I changed the layer indexing to be done by uuid. But, as it turns out, querying a layer from the viewer like this viewer.layers[uuid] isn't possible, whereas viewer.layers[name] is - which means that we should definitely index by name. I'm reverting this elsewhere and send you a PR to this. Then it's good to go, where I am concerned 👍

[jo-mueller]

Hi @zoccoler , I looked into the failing test and it's basically because some of the tests only work with "napari>=0.6.5". Wei could either adjust the test to check for the correct version or configure the tests to install the newest napari version which would work?

Maybe introducing a dependency pin like this in the testing configuration would do the trick?

[project.optional-dependencies]
testing = [
    "tox",
    "pytest",  # https://docs.pytest.org/en/latest/contents.html
    "pytest-cov",  # https://pytest-cov.readthedocs.io/en/latest/
    "pytest-qt",  # https://pytest-qt.readthedocs.io/en/latest/
    "napari>=0.6.5",
    "pyqt5",
]

[jo-mueller]

Ok, took me a bit of digging but I think I found the issue that let's tests fail here. Apparently it's a bit deeper in how highlighting is implemented right now 😬

Essentially, highlighting triggers an infinite loop. If you try it manually from the viewer, you experience a noteable lag between the selection click and the point on the biaplotter canvas showing up as highlighted. The loop is this:

1. Highlighting data calls _highlight_data
2. _highlight_data calls bin_alpha setter
3. bin_alpha setter calls _refresh() and _colorize()
4. _refresh() calls highlighted = None
5. This triggers the highlight data setter --> cycle continues.

[zoccoler]

Thanks for the deep testing @jo-mueller ! I will try to reproduce it and check how to fix it.

[jo-mueller]

Thanks for the deep testing @jo-mueller ! I will try to reproduce it and check how to fix it.

Thank me and the copilot ^^"

Also, just a thought: What if we re-write the highlighter into some sort of pciker-selector? Here on the clusters-plotter we could just add the callback to the _focus_objectfunction only if the picker selector is active. Over at the biaplotter we could reuse a lot of the already existing (and well-tested) selection mechanic to avoid such deadlocks.

[zoccoler]

Do you mean like a new kind of selector, with a button in the toolbar? It is a good idea! I mean, it is a bit less smooth to use it, but it also wouldn't lead to unexpected behavior from the user side.

[jo-mueller]

Do you mean like a new kind of selector, with a button in the toolbar?

Jup! I think a lot of the underlying logic would be the same (select, unselect, etc). The cool thing is that we could reuse the already existing signals and still introduce the kind of single-object highlighting we want. For instance, would it be possible to check in the _colorize() method of the biaplotter whether the selection was done by a picker and then introduce a specific highlighting?

There's probably a lot to be learnt from the Mastodon UI :)

[zoccoler]

For instance, would it be possible to check in the _colorize() method of the biaplotter whether the selection was done by a picker and then introduce a specific highlighting?

I wouldn't check inside _colorize() at first, but that should be doable with some signal/slot mechanisms if needed. Alright, it seems this would be a better way to go.