Skip to content

add LAYER_SELECTED_DATA_CLUSTER_ID to cluster from layer space #366

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

add LAYER_SELECTED_DATA_CLUSTER_ID to cluster from layer space #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b230bf6
add test for layer selection in plotter widget
ClementCaporal Feb 27, 2025
6985fa3
add event connection to update selected data for Point Layers in Plot…
ClementCaporal Feb 27, 2025
497e14c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 27, 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
18 changes: 18 additions & 0 deletions src/napari_clusters_plotter/_new_plotter_widget.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,24 @@ def _on_update_layer_selection(
for layer in self.layers:
layer.events.features.connect(self._update_feature_selection)

# if layer is a Point Layer
if type(layer) is napari.layers.Points:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe other layer type are eligible?

Copy link
Collaborator

@jo-mueller jo-mueller Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@jo-mueller jo-mueller Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

layer.selected_data.events.items_changed.connect(
lambda selected_data: self._update_layer_selected_data_feature(
layer, selected_data
)
)

def _update_layer_selected_data_feature(
self, layer: napari.layers.Points, selected_data: np.ndarray
) -> None:
"""
Update the layer selected_data to feature.
"""
cluster = np.zeros(layer.data.shape[0])
cluster[list(selected_data)] = 1
layer.features["LAYER_SELECTED_DATA_CLUSTER_ID"] = cluster
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@ClementCaporal ClementCaporal Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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 👼


def _update_feature_selection(
self, event: napari.utils.events.Event
) -> None:
Expand Down
24 changes: 24 additions & 0 deletions src/napari_clusters_plotter/_tests/test_plotter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,27 @@ def test_cluster_memorization(make_napari_viewer, n_samples: int = 100):
plotter_widget.plotting_widget.active_artist.color_indices
== cluster_indeces
)


def test_layer_selection(make_napari_viewer, n_samples: int = 100):
from napari_clusters_plotter import PlotterWidget

viewer = make_napari_viewer()
_, layer2 = create_multi_point_layer(n_samples=n_samples)

# add layers to viewer
viewer.add_layer(layer2)
plotter_widget = PlotterWidget(viewer)
viewer.window.add_dock_widget(plotter_widget, area="right")

# select last layer and create a random selection on the layer
viewer.layers.selection.active = layer2
selection = np.random.randint(0, 1, len(layer2.data))
layer2.selected_data = selection

assert "LAYER_SELECTED_DATA_CLUSTER_ID" in layer2.features.columns

# make sure that the cluster selection is the same
cluster = np.zeros(layer2.data.shape[0])
cluster[list(selection)] = 1
assert np.all(layer2.features["LAYER_SELECTED_DATA_CLUSTER_ID"] == cluster)
Comment on lines +113 to +134
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)