napari-clusters-plotter icon indicating copy to clipboard operation
napari-clusters-plotter copied to clipboard

Deselect clusters

Open thorstenwagner opened this issue 1 year ago • 7 comments

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

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

vokoscreenNG-2023-12-18_16-11-03

thorstenwagner avatar Dec 18 '23 15:12 thorstenwagner

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 :warning:
napari_clusters_plotter/_plotter.py 28.57% 5 Missing :warning:
napari_clusters_plotter/_plotter_utilities.py 0.00% 3 Missing :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 18 '23 15:12 codecov[bot]

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

thorstenwagner avatar Dec 19 '23 16:12 thorstenwagner

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.

stefanhahmann avatar Dec 19 '23 17:12 stefanhahmann

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 avatar Dec 21 '23 15:12 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.

thorstenwagner avatar Jan 15 '24 08:01 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?

Cryaaa avatar Jan 15 '24 08:01 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?

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

thorstenwagner avatar Jan 15 '24 08:01 thorstenwagner