Skip to content

Escape empty selection#379

Merged
jo-mueller merged 11 commits intov0.9.0from
escape-empty-selection
Apr 23, 2025
Merged

Escape empty selection#379
jo-mueller merged 11 commits intov0.9.0from
escape-empty-selection

Conversation

@jo-mueller
Copy link
Collaborator

@jo-mueller jo-mueller commented Apr 14, 2025

Relevant for #378

Hi @Cryaaa , this solves your problem. In essence, the logic on the napari side is safe, the problem was here: The case of an empty selection (after a previous non-empty selection) is not escaped anywhere in the widgets, so that's a pretty serious flaw you found :)

I added specific _clean_up methods that handle the teardown of any remaining references to previous layers as well as escape statements that check whether there is data in the viewer before trying to do anything. I wasn't able to completely clean up the plotter itself (i.e., the plotted data), but that's something to fix over at the biaplotter (see BiAPoL/biaplotter#45).

Copilot description

This pull request introduces several changes to the napari_clusters_plotter package, focusing on enhancing the handling of empty layer selections, adding new functionalities, and improving test coverage. The most significant changes include the implementation of _clean_up methods, adjustments to handle empty input data, and the addition of new tests.

Enhancements to handling empty layer selections:

New functionalities:

Test improvements:

These changes aim to improve the robustness and reliability of the napari_clusters_plotter package.

@jo-mueller jo-mueller added bug Something isn't working enhancement New feature or request test Improve, fix or add tests for the clusters-plotter labels Apr 14, 2025
@jo-mueller jo-mueller added this to the v0.9.0 milestone Apr 14, 2025
@Cryaaa
Copy link
Collaborator

Cryaaa commented Apr 15, 2025

@jo-mueller,
very nice fix, the plotter definitely now loses the features but it does not fix the issue of manual cluster ID going missing. What's super weird is that on my end if I inspect the features of the plotter or even the features of the surface layer I can see that the manual cluster ID column is there but it just doesn't get used as the plot color and it does not appear in the hue combo box. No idea what is wrong there> Is it maybe a napari version thing? Mine is 0.5.3...

@jo-mueller
Copy link
Collaborator Author

Hi @Cryaaa , unfortunately I can't reproduce your issue. For me mentioned missing MANUAL_CLUSTER_ID is always there after adding a surface layer to the viewer 🤔

I don't think it's due to the hardcoded surface_layer.features, as that is supported since napari 0.5.6...what code are you using for testing? Is it the demo_new_plotter.ipynb notebook?

@codecov
Copy link

codecov bot commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 89.13043% with 5 lines in your changes missing coverage. Please review.

Project coverage is 84.20%. Comparing base (b9fb712) to head (dc23f3d).
Report is 28 commits behind head on v0.9.0.

Files with missing lines Patch % Lines
src/napari_clusters_plotter/_algorithm_widget.py 72.72% 3 Missing ⚠️
src/napari_clusters_plotter/_new_plotter_widget.py 85.71% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           v0.9.0     #379      +/-   ##
==========================================
+ Coverage   83.11%   84.20%   +1.09%     
==========================================
  Files          11       11              
  Lines         752      842      +90     
==========================================
+ Hits          625      709      +84     
- Misses        127      133       +6     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jo-mueller jo-mueller merged commit 79fe4bd into v0.9.0 Apr 23, 2025
19 checks passed
@jo-mueller jo-mueller deleted the escape-empty-selection branch April 23, 2025 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request test Improve, fix or add tests for the clusters-plotter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants