connect napari-sided object selection in plotter widget

[ClementCaporal]

Follow-up on #366

---

Apologies for the delay in getting back to you!

Since I couldn't copy-paste your comments directly, here's a summary of what I’ve addressed based on your feedback:

• I incorporated your _is_selectable and _get_selected_objects as private functions and applied them where needed.
• I updated the test to check the color indices directly from the current plotter.
• Regarding the variable name _VIEWPORT_CLUSTER_ID: I kept the original name SELECTED_DATA_LAYER_CLUSTER_ID to stay consistent with the naming used in the napari layer selected_data. I don't have a strong opinion on this, this is just a proposition, I can change it.

---

This PR is not ready yet (and it doesn't pass all the tests) because :
I’m still facing an issue related to selection on the labels layer that I think are out of scope of this PR.

To handle label selection, we need to listen to the selected_label event:
https://github.com/napari/napari/blob/86e6d27c547f3fad22ddfe8835c4869d3f0931ed/src/napari/layers/labels/labels.py#L382

From there, we update the layer features and emit self.plot_needs_update. This eventually leads to updating layer.colormap, which unfortunately triggers the selected_label event again:
https://github.com/napari/napari/blob/86e6d27c547f3fad22ddfe8835c4869d3f0931ed/src/napari/layers/labels/labels.py#L571

This creates an unintended recursive loop.

I think this might be a bug on napari’s side (it's unclear to me why changing the colormap should emit a selected_label event). Unless I’ve missed something, I’m will open an issue on their repo based on your review.

Thanks again for your time and for the excellent plugin!

[jo-mueller]

This doesn't necessarily have to be a member of the PlotterWidget - none of the functionality relies on self. Originally, some other functionality in here (e.g., _set_layer_color) weren't member functions but I think that was lost somewhere along the way.

Still - moving it to _utilities.py could help to keep this widget a bit cleaner here :)

[ClementCaporal]

Ok I will move it!

[jo-mueller]

Not sure - does it make sense to iterate over all the layers and collect the selected objects inside this function? That way, you could save yourself the lambda callback connection above. Somethhing like this:

for layer in self.layers:
  selected_data = self._get_selected_objects(layer)
  cluster = np.zeros(len(layer.features))
  cluster[list(selected_data)] = 1
  # set categorical to be selectable in "Hue" dropdown
  layer.features["SELECTED_DATA_LAYER_CLUSTER_ID"] = pd.Categorical(cluster)

edit: Avoiding the lambda function would probably also make pre-commit happy :)

[jo-mueller]

Hi @ClementCaporal ,

just gave it a try, it looks pretty much how I envisioned it to work for selectable objects :)

This creates an unintended recursive loop.

I was wondering why it was so slow for labels 😅

From there, we update the layer features and emit self.plot_needs_update. This eventually leads to updating layer.colormap, which unfortunately triggers the selected_label event again:

Maybe we could just mute/block this event while updating the colormap? I just tried to add the following into the _set_layer_color function and it worked:

        elif isinstance(layer, napari.layers.Labels):
            from napari.utils import DirectLabelColormap

            from ._utilities import _get_unique_values

            # Ensure the first color is transparent for the background
            colors = np.insert(colors, 0, [0, 0, 0, 0], axis=0)
            color_dict = dict(zip(_get_unique_values(layer), colors))
            layer.events.selected_label.block()  # avoid triggering the infinite loop
            layer.colormap = DirectLabelColormap(color_dict=color_dict)
            layer.events.selected_label.unblock()  # restore functionality for whatever purpose it serves