Skip to content

Conversation

timtreis
Copy link
Member

@timtreis timtreis commented Jul 14, 2024

Note: the only existing utils function was using a different testing pattern, which I now aligned with the usual one. But therefore there's an additonal image in this PR

@timtreis timtreis linked an issue Jul 14, 2024 that may be closed by this pull request
@timtreis timtreis added enhancement New feature or request priority: low utils 🔧 Anything related to util functions labels Jul 14, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.35%. Comparing base (91e5726) to head (9ebc9ad).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #302      +/-   ##
==========================================
+ Coverage   80.18%   80.35%   +0.17%     
==========================================
  Files          11       11              
  Lines        1736     1746      +10     
==========================================
+ Hits         1392     1403      +11     
+ Misses        344      343       -1     
Files Coverage Δ
src/spatialdata_plot/pl/utils.py 70.21% <100.00%> (+0.44%) ⬆️

@LucaMarconato
Copy link
Member

Amazing! Ready for review?

@timtreis timtreis requested a review from LucaMarconato July 14, 2024 16:39
@timtreis timtreis self-assigned this Jul 14, 2024
@timtreis
Copy link
Member Author

Added the function @LucaMarconato @giovp. Probably best to add it to one of the notebooks to illustrate usage - which one do you think would be best?

@timtreis timtreis requested a review from giovp July 14, 2024 16:40
@LucaMarconato
Copy link
Member

Thanks! I am working on the Visium HD notebook right now, so I can add it there 😊

@timtreis timtreis requested a review from melonora July 14, 2024 20:11
Copy link
Contributor

@melonora melonora left a comment

Choose a reason for hiding this comment

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

LGTM! Maybe one consideration is to add a parameter that automatically does this for the user e,g, zero_transparent, but this could be added in a different PR. Also this PR does make it a bit more difficult for view configurations as we are dealing with different alpha values. May have to think of serialization of the colormap values.

@timtreis
Copy link
Member Author

Yeah, based on #242 we decided not to include it as an argument but like this. But I agree, eventually, we should probably just serialise the data. I think ggplot f.e. also does it like this when generating the plot

@timtreis timtreis merged commit da1f84e into main Jul 15, 2024
@timtreis timtreis deleted the feature/issue242-modify-colormap-so-that-0-values-have-alpha=0 branch July 15, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: low utils 🔧 Anything related to util functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

modify colormap so that 0 values have alpha=0
4 participants