-
Notifications
You must be signed in to change notification settings - Fork 17
Fix setting vmin vmax #307
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code lgtm, but can you add the fix to the changelog? Also, what do you think of plotting a 2x1 grid with the non-clipped and clipped image so it's visually easy see the difference?
added to changelog. Regarding 2 x 1 grid, I would if ncols was working xD. I don't want to create a fig object here now. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #307 +/- ##
==========================================
+ Coverage 80.21% 80.35% +0.13%
==========================================
Files 11 11
Lines 1739 1746 +7
==========================================
+ Hits 1395 1403 +8
+ Misses 344 343 -1
|
Why the aversion against the fig object? Used it f.e. in the 0-transparency cmap PR: https://github.com/scverse/spatialdata-plot/pull/302/files#diff-59872f651a8fef967621d774a8b7441219dc474f1d9604424c54fe19a162bbfd |
I would just solve the issue with ncols as well:) like I would not add more code in a test because of ncols, though behavior in this case is a bit tricky |
added the baseline to the test for now and will open an issue on ncols as argument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for adjusting the test!
closes #303
Fixes the problem in #303. Main problem is that clipping for the
Normalize
object is set toFalse
by default inmatplotlib
. I have set this value toTrue
and added tests. I don't think we currently have a usecase for setting it toFalse
, but feel free to comment if there is a usecase for us currently.