Update Colorizer/ColorizingArtist to work with MultiNorm#30511
Update Colorizer/ColorizingArtist to work with MultiNorm#30511story645 merged 4 commits intomatplotlib:mainfrom
Conversation
77bc51a to
9044189
Compare
9b9aaf3 to
525314e
Compare
|
Thank you for the feedback @QuLogic and apologies for my sloppy mistakes :) |
improved testing for colorizer+multinorm Apply suggestions from code review Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com> updates based on feedback from @QuLogic
525314e to
23fb63c
Compare
eb37db8 to
d5c43e0
Compare
QuLogic
left a comment
There was a problem hiding this comment.
As I mentioned on the call, I think this looks okay from a low-level point-of-view, but I am at this point uncertain as to the high-level status. I hope someone more familiar with the current state can confirm.
PS, @trygvrad if you have some time to pop in to the call, it would be good to just re-iterate the current state succinctly, as I'm not sure everyone is up-to-speed right now.
story645
left a comment
There was a problem hiding this comment.
Sorry it took so long to get to this.
lib/matplotlib/colorizer.py
Outdated
| if norm and (xx.max() > 1 or xx.min() < 0): | ||
| raise ValueError("Floating point image RGB values " | ||
| "must be in the 0..1 range.") | ||
| "must be in the 0..1 range") |
There was a problem hiding this comment.
| "must be in the 0..1 range") | |
| "must be in the [0,1] range") |
teenyist nit but I find interval notation clearer
There was a problem hiding this comment.
FYI: This required a change to an existing test test_colors.py→test_scalarmappable_nan_to_rgba(bytes) which had a regex for the string '0..1 range'
| elif n_components == 2: | ||
| raise ValueError("Invalid data entry for multivariate data. The data" | ||
| " must contain complex numbers, or have a first dimension 2," | ||
| " or be of a dtype with 2 fields") |
There was a problem hiding this comment.
I'm not following what 2 components means in this context
There was a problem hiding this comment.
We had a discussion when declaring MultiNorm that we should refer to the different channels/variates as components, so MultiNorm has the property n_components which is the number of channels/variates/components.
In the following example it would be 2, because the colormap supports 2 channels, and we provide 2 datasets.
plt.imshow((A, B), cmap='BiOrangeBlue')
There was a problem hiding this comment.
Thanks! Sorry for forgetting this. :/
No worries, thank you for getting around to it :) |
Co-authored-by: hannah <story645@gmail.com>
cf9296d to
9409d4c
Compare
story645
left a comment
There was a problem hiding this comment.
Comments but definitely not blocking, will leave unmerged for a day but they can also be addressed on followup. Thanks for all your patience and persistence.
| elif n_components == 2: | ||
| raise ValueError("Invalid data entry for multivariate data. The data" | ||
| " must contain complex numbers, or have a first dimension 2," | ||
| " or be of a dtype with 2 fields") |
There was a problem hiding this comment.
Thanks! Sorry for forgetting this. :/
|
@story645 I added the test we discussed, please take a look at the last commit :) |
8745b81 to
6934853
Compare
|
doc build failures seem to be intersphinx related |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
EDIT: closes #30511
This PR continues the work of #28658 and #28454, #29876, aiming to close #14168. (Feature request: Bivariate colormapping)
This PR allows Colorizer and ColorizingArtist to work with
MultiNormandBivarColormapandMultivarColormapi.e. this PR will allow:
Features not included in this PR:
axes.imshow(...),axes.pcolor(...), andaxes.pcolormesh(...)`PR checklist