add LAYER_SELECTED_DATA_CLUSTER_ID to cluster from layer space

[ClementCaporal]

Hello,

Attempt to close #363

In this PR I propose to add to v.0.9.0 the point layer features column LAYER_SELECTED_DATA_CLUSTER_ID.

The event connection is added during _on_update_layer_selection (and never removed?). This allow the user to select points in the layer to appear on the plotter canvas through features.

---

v0.9.0 seems amazing! For example, the ability to select several layers and to plot them on the same plotter canvas is something I didn't know I needed.

I feel I didn't understand how to change the color_indices by using another CLUSTER_ID than MANUAL_CLUSTER_ID but maybe it is not implemented yet?

I tried to keep the PR minimal because I am unsure about how v0.9.0 will be developed, but I can of course change the implementation / names.

Thank you for your time

[ClementCaporal]

Maybe other layer type are eligible?

[jo-mueller]

Labels layers have layer.selected_label, that could work, too. Maybe we could add a property is_selectable(self) to the plotter widget that will return True or False if the layer supports some sort of interactive selection. We could then hide the if/else check for the kind of layer in that property/helper function.

It could look something like this:

def is_selectable(self, layer) -> bool:
  if isinstance(layer, napari.layers.Points) or isinstance(layer, napari.layers.Labels)
    return True
  else:
    return False

def get_selected_objects(self, layer) -> List[int]:
  """
  Retrieve id of selected object on napari canvas"
  """
  if isinstance(layer, napari.layers.Points):
    return layer.selected_data
  elif isinstance(layer, napari.layers.Labels):
    return [layer.selected_label]

At this point we could then simply add a check

if self.is_selectable(layer):
  selected_items = self.get_selected_objects(layer)

and then pass it on to the _update_selected_layer... function, where we would have to add the same check for the kind of layer. That would make it a bit easier extensible down the road.

[ClementCaporal]

Nice this a neat solution! Do you want to commit on the branch or do you want me to add your snippet?

[jo-mueller]

I'm too dumb right now to send a PR to your branch but I'd basically have it ready to a degree. If you want I can also paste it here and you just add me as a contributor to the commit 🤷

You'd basically need this in the class definition:

class PlotterWidget(BaseWidget):
    """
    Widget for plotting data from selected layers in napari.

    Parameters
    ----------
    napari_viewer : napari.Viewer
        The napari viewer to connect to.
    """

    selectable_layer_types = [
        napari.layers.Labels,
        napari.layers.Points,
    ]

and these convenience functions:

    def is_selectable(self, layer: napari.layers.Layer) -> bool:
        """
        Check if the layer is selectable.
        """
        if type(layer) in self.selectable_layer_types:
            return True
        else:
            return False

    def get_selected_objects(self, layer: napari.layers.Layer) -> List[int]:
        """
        Retrieve id of selected object on napari canvas"
        """
        if isinstance(layer, napari.layers.Points):
            return list(layer.selected_data)
        elif isinstance(layer, napari.layers.Labels):
            return [layer.selected_label]

    def get_selection_event(self, layer: napari.layers.Layer):
        """
        Get the selection event for the layer.
        """
        if isinstance(layer, napari.layers.Points):
            return layer.selected_data.events.items_changed
        elif isinstance(layer, napari.layers.Labels):
            return layer.events.selected_label

the respective code in the _on_update_layer_selection function would then look like this:

        self.layers = list(self.viewer.layers.selection)
        self._update_feature_selection(None)

        for layer in self.layers:
            layer.events.features.connect(self._update_feature_selection)

            # if objects in the layer can be selected interactively in napari
            if self.is_selectable(layer):
                event = self.get_selection_event(layer)
                event.connect(self._update_layer_selected_data_feature(layer))

    def _update_layer_selected_data_feature(
        self, layer: napari.layers.Layer
    ) -> None:
        """
        Update the layer selected_data to feature.
        """
        selected_data = self.get_selected_objects(layer)
        cluster = np.zeros(len(layer.features))
        cluster[list(selected_data)] = 1
        layer.features["LAYER_SELECTED_DATA_CLUSTER_ID"] = cluster

[zoccoler]

Hi @ClementCaporal ! Hey, thanks for your interest in the plugin and for already helping out with a PR! 🎉
Our team is at the moment preparing contents for a course and we will resume 0.9.0 efforts early next month. Maybe before, but I can't promise.
I hope it is OK for you if we review this by then! Thanks for your patience and enthusiasm!

[ClementCaporal]

Hello @zoccoler ,

Thank you for your message! I hope you take your time and enjoy your course preparation.

[jo-mueller]

Hi all - I am only seeing this now, will look at it soon hopefully! Thanks @ClementCaporal for the PR already!

Edit: removed all tag 😅

[ClementCaporal]

I don't imagine how often @ ALL is tagged. Might be like famous et Al. contributor https://scholar.google.nl/citations?user=qGuYgMsAAAAJ&hl=en

[jo-mueller]

Maybe one suggestion for a clean way to check whether the selection from the viewer is forwarded to the plotter widget: The plotter widget has an item that contains the actual plotter/canvas/stuff where you'll find a reference to the ids of each object's currently selected cluster

widget.plotting_widget.active_artist.color_indices

which comes from here. Essentially, you could write the test so that it checks whether this property has actually been updated.

[ClementCaporal]

Thank you for the review @jo-mueller,

I agree that the test should check if the selection can update color_indices.
However, in v0.9.0 I don't understand how to update the cluster that is shown in the plotting_widget.

From my understanding the current implementation only allows MANUAL_CLUSTER_ID to update the color_indices here

napari-clusters-plotter/src/napari_clusters_plotter/_new_plotter_widget.py

Lines 161 to 164 in 73192f5

	if "MANUAL_CLUSTER_ID" in features.columns:
	self.plotting_widget.active_artist.color_indices = features[
	"MANUAL_CLUSTER_ID"
	].values

Should this PR wait for a method select_active_artist_cluster (is this the goal of Hue dropdown?) that would allow to select any features that have the "CLUSTER_ID" name in it and pass it to the self.plotting_widget.active_artist.color_indices ?

Should I manually update in the test the widget.plotting_widget.active_artist.color_indices and then check it worked? (I feel this solution is cheating)

[jo-mueller]

I would change the name of the column to something like _VIEWPORT_CLUSTER_ID, the notable part being the leading _ - which would allow us to ignore the feature in the BaseWidgets .get_features method to prevent it from showing up as a plotable feature in the dimensionality/clustering widgets

[ClementCaporal]

Thank you for the review @jo-mueller !

• Concerning the name: I don't have a strong opinion about it. Does VIEWPORT refer to a specific napari terminology ? It seems to be called "CANVAS" in the documentation(https://napari.org/dev/tutorials/fundamentals/viewer.html#canvas). I used the LAYER_SELECTED_DATA in reference with point_data.selected_data
• Concerning the _ : I understand why it is important to keep the "plotable" feature less crowded. However, I have some use-cases where I want to cluster with all possible features, including previous cluster. I was also thinking about opening an issue to allow the get_features to add the points "axis-*" values 👼

[jo-mueller]

Hi @ClementCaporal , just a quick update for you here: We are working on a more stratified way of handling colors so that cololring points in the plotter by arbitrary features will be easier. To be compliant with the new functionality, it should be enough if you add the selected items from the point cloud as a new feature to the respective layer's feature (as you already do it) - you'll only have to make sure that it's a column of type categorical` so that the plotter handles it correctly.

[ClementCaporal]

Ok I will add the categorical type constrain to the PR and update also the previous feedback.
You can also update the branch if I take too long (writing thesis..)

[jo-mueller]

@ClementCaporal seems like I accidentally closed this PR by deleting the target branch 😬 Please don't interpret it as if your contribution isn't wanted!

[ClementCaporal]

Hello @jo-mueller,
Thank you for letting me know.
No problem, It happened to me once!

I plan to update (reopen) the PR this weekend.