Skip to content
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.

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

Coloring by features as well as cluster #257

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Cryaaa
Copy link
Collaborator

@Cryaaa Cryaaa commented Jun 8, 2023

This PR will address #209. Currently I have implemented it so that the features are also available in the dropdown for cluster ID and the colormap can be selected even when in scatterplot mode.

When features are plotted a feature layer is created. One problem was that this layer can be hidden if a cluster layer is created afterwards and vice versa. My way of solving this problem is to keep the feature layer as the top layer and changing the visibility of the layers for feature or cluster visualization depending on what is plotted. I'm not sure if this is the best solution and would be grateful if anyone tests it.

Furthermore, we would have to add a colorbar so that the colors for the features can be interpreted.

I would be happy if someone would take a look at the code or try it out as it does add quite a few lines, but I could not think of a more elegant solution without rewriting a lot of the other code..

Anyway here is a small video demonstrating how it works at the moment:
https://github.com/BiAPoL/napari-clusters-plotter/assets/65285466/153a0d8d-a21a-450b-9b4e-88fde6926cd7

We still have a few bugs: The axis label text changes color when plotting features.
Implementing the actual visualisation
@Cryaaa Cryaaa requested a review from haesleinhuepf June 8, 2023 15:23
@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #257 (070769a) into main (40f3f43) will decrease coverage by 2.02%.
The diff coverage is 17.46%.

@@            Coverage Diff             @@
##             main     #257      +/-   ##
==========================================
- Coverage   75.82%   73.81%   -2.02%     
==========================================
  Files          14       14              
  Lines        1663     1726      +63     
==========================================
+ Hits         1261     1274      +13     
- Misses        402      452      +50     
Impacted Files Coverage Δ
napari_clusters_plotter/_plotter.py 61.36% <15.90%> (-5.12%) ⬇️
napari_clusters_plotter/_plotter_utilities.py 63.63% <21.05%> (-6.00%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jo-mueller
Copy link
Collaborator

Hi @Cryaaa ,

supercool stuff and thanks for working on it! What I am currently a bit worried about (not specifically about this PR, but about the clusters-plotter in general) - creating the cluster visualization (be it of the cluster or the intensity vis.), the code is already all over the place and I am wondering whether or not to put the code for creating the cluster view (of cluster or features) in its own place.

There is a pull request to napari-skimage-regionprops that unifies the creation of featuremaps for labels data and a bunch of other not currently supported layers (points, surfaces, vectors).

On top of that, PR #229 introduces some functionality to create the cluster maps in a central place to make the code a bit cleaner. I am leaning a bit towards first having this go through and then move the feature vis. forward so we don't have to later on add support for all layer types when moving this forward.

Looking forward to hearing your opinion!

@Cryaaa
Copy link
Collaborator Author

Cryaaa commented Nov 13, 2023

Hey @jo-mueller ,
Just went through the different PRs and definitely agree that the big changes should be implemented before this one. I also strongly agree that the code for generating the cluster view is super chaotic and I also was thinking about how to deal with this to make the plotter widget more readable... I would love to have a discussion with everyone still actively working on it so we could maybe figure something out which makes the development easier, as I did not want to just refactor the code on my own and possibly break things other people have put into place!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants