Skip to content

Conversation

@Sonja-Stockhaus
Copy link
Collaborator

Fix coloring of shapes, labels and points when there are nan values in the vectors to color by

@Sonja-Stockhaus Sonja-Stockhaus linked an issue Mar 5, 2025 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2025

Codecov Report

❌ Patch coverage is 72.97297% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.02%. Comparing base (0d4239b) to head (9244f4f).

Files with missing lines Patch % Lines
src/spatialdata_plot/pl/utils.py 43.75% 27 Missing ⚠️
src/spatialdata_plot/pl/render.py 83.21% 23 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
- Coverage   83.75%   83.02%   -0.73%     
==========================================
  Files           8        8              
  Lines        2555     2681     +126     
==========================================
+ Hits         2140     2226      +86     
- Misses        415      455      +40     
Files with missing lines Coverage Δ
src/spatialdata_plot/pl/render.py 88.24% <83.21%> (-0.52%) ⬇️
src/spatialdata_plot/pl/utils.py 78.11% <43.75%> (-1.24%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Sonja-Stockhaus Sonja-Stockhaus marked this pull request as ready for review March 26, 2025 17:18
Comment on lines -822 to -1152
if np.any(color_source_vector.isna()):
cell_id[color_source_vector.isna()] = 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In seg, the value 0 depicts the background, so this would lead to the bg being mapped to the NaN color
The actual label(s) with na in the color_source_vector don't have their id in cell_id anymore, so they're mapped to nothing! => would look like background

@MeyerBender
Copy link
Contributor

Hi @timtreis, just wondering what the status of this is, is there a reason why this hasn't been merged yet? I constantly run into this issue, so this would be a very useful thing to have in the package.

@timtreis
Copy link
Member

Hey @MeyerBender, sorry, will resolve the merge conflicts and get it released today :)

@timtreis
Copy link
Member

@MeyerBender, there's an edge case that I haven't figured out how to solve yet. Will be a bit delayed

@timtreis
Copy link
Member

@MeyerBender can you try this branch?

@MeyerBender
Copy link
Contributor

@timtreis Thanks a lot for your efforts! Just tried it on one of my datasets containing NAs and it worked flawlessly.

@timtreis
Copy link
Member

Great! I discovered two other minor but related issue/inconsistencies which are also fixed in this. I'll have to clean it up a bit but should be out next week. Feel free to give code review if you feel comfortable doing so.

@MeyerBender
Copy link
Contributor

Sounds great, happy to have a look

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.

Categorical color obs column crashes the plotting

5 participants