Deselect clusters

[thorstenwagner]

This is my attempt for an implementation to deselected clusters (#289):

If you keep control pressed and click on a cluster, it gets deleted.

[codecov]

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (db9aea7) 77.33% compared to head (0217a51) 76.57%.

Files	Patch %	Lines
napari_clusters_plotter/_Qt_code.py	22.22%	21 Missing ⚠️
napari_clusters_plotter/_plotter.py	28.57%	5 Missing ⚠️
napari_clusters_plotter/_plotter_utilities.py	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #291      +/-   ##
==========================================
- Coverage   77.33%   76.57%   -0.76%     
==========================================
  Files          16       16              
  Lines        1897     1921      +24     
==========================================
+ Hits         1467     1471       +4     
- Misses        430      450      +20     

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

[stefanhahmann]

To me this looks definitely like a cool feature to be added to the clusters plotter.

However, I checked out this branch and tried the feature in a Windows environment and could not get it to work.

For a test data test, I followed this workflow with Napari built-ins:

Then I tried to generate 2 clusters with Shift + Mouse Drag (which works as always) and then tried to delete only 1 cluster with Control + Left Click, which delete all clusters and not only the cluster I was pointing to:

Am I doing something wrong?

[thorstenwagner]

Hi! Thanks for testing. The implementation is only done for histograms, not for point clouds

[stefanhahmann]

Hi! Thanks for testing. The implementation is only done for histograms, not for point clouds

Okay. Good to know.

I tested with histograms and there it works as expected. The only thing, I would suggest: when using Ctrl + Click outside any cluster, it would find it more intuitive, if nothing changes, instead of the deletion of all clusters. The latter is the standard behavior, when just clicking. In a File Explorer (e.g. in Windows), if you select multiple files/folders with Ctrl + Click and then click in an empty area, the selection of files/folder does not change (i.e. is not cleared). I would recommend to follow to this pattern, since it might be intuitive to users, since they are used to this pattern.

From a user perspective, it would of course be nice to have the same behavior also in point scatter plots. Users may interpret it otherwise not as a missing feature for the scatter plot, but as a bug and report it as such.

Regarding the code: I do not feel confident enough to give feedback regarding the code.

[thorstenwagner]

Hi!

Now nothing happens when control is pressed. Good point!

I agree that it should also work for point scatter plots. However, the current implementation depends on the histograms. We can implement a more generalized mechanism, by saving the individual paths for the lassos, but the current implementation depends on the histogram overlay and is not easy to generalize.

[thorstenwagner]

Any chances to see that merged without the additional implementation for scatter + histograms? As far as I can see this would require a bigger refactoring.

[Cryaaa]

Hey @thorstenwagner,
I am unsure if we should add this feature here if it is only a histogram specific one. I generally love the functionality but we are currently also planning a rehaul of the code structure and maybe it just might be better to add a more generalized version then.. maybe @zoccoler and @jo-mueller can also chime in?

[thorstenwagner]

Hey @thorstenwagner, I am unsure if we should add this feature here if it is only a histogram specific one. I generally love the functionality but we are currently also planning a rehaul of the code structure and maybe it just might be better to add a more generalized version then.. maybe @zoccoler and @jo-mueller can also chime in?

Its a very useful but not critical feature for me. So its fine for me if we add this later.