Skip to content

Added utils function for 0-transparent cmaps #302

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

Merged

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
5 checks passed
@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