-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
VectorMappable with data type objects #28428
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
base: main
Are you sure you want to change the base?
VectorMappable with data type objects #28428
Conversation
attn @timhoffm, can you take a quick skim to see if this is a reasonable API and plan/share any concerns? |
I rebased this code so that it correctly builds on #28454 |
8ae3a0d
to
6fb5884
Compare
6fb5884
to
e889929
Compare
e889929
to
81181c9
Compare
The current scalar mapping works like this. How do VectorMappable and the new colormap types change the picture? Edit: I think the OP is sufficient. I‘ll have a closer look. |
81181c9
to
f11c3bf
Compare
The This PR does not contain changes to the plotting methods, but it is intended to go something like this. class Axes:
def pcolor(self, data, cmap = None, norm = None, vmin = None, vmax = None):
cmap = colors._ensure_cmap(cmap)
data, norm, vmin, vmax = colors._ensure_multivariate_params(cmap.n_variates, data, norm, vmin, vmax) Which transforms *I prefer an input signature of (cmap.n_variates, n, m) for multivariate data, but we could use (n, m, cmap.n_variates) instead. Edit: @timhoffm Thank you for taking a look, I didn't see your edit before I made the additional figure :D |
It is not quite clear to me what the distinction between between the Having a
The alternative, as this PR suggests, is by simply attaching a list of norms to However, (all of) you are more experienced to evaluate what makes code more easy to maintain than I am, and if you think a
I have been thinking we need to determine how the equivalent to
To me, the intuitive behavior has always been that each axis has a separate norm: ax.imshow((A, B), cmap='BiOrangeBlue')
# or
ax.imshow((A, B), cmap='BiOrangeBlue', norm='LogNorm') should give two separate norms with independent limits. I further believe that the intuitive action for a user to take if they want identical limits on both axis is to simply set the limits: ax.imshow((A, B), cmap='BiOrangeBlue', vmin = -1, vmax = 2) However, advanced users might want the behavior you describe above, a shared norm, and we should therefore support the following: n = mpl.colors.Normalize()
ax.imshow((A, B), cmap='BiOrangeBlue', norm=(n, n)) Where both axis are explicitly declared to have the same norm. n = mpl.colors.Normalize()
norm.vmin = np.min((A, B))
norm.vmax = np.max((A, B))
ax.imshow((A, B), cmap='BiOrangeBlue', norm=(n, n)) NOTE: I made a commit this morning that explicitly forbids the call following call signature: n = mpl.colors.Normalize()
ax.imshow((A, B), cmap='BiOrangeBlue', norm=n)
# → ValuError('When using a colormap with more than one variate, norm must be None,
# a valid string, a sequence of strings, or a sequence of mpl.Colors.Normalize objects.' because it I found it unclear if this should be a shared norm (shared limits) or if the norm should be duplicated (independent limits), and I believe we should only accept input where the users intent is clear. @timhoffm As I understand it, the weekly development meeting today is cancelled, but I will try to be there next week. |
Thanks for the detailed answer, I'll have a closer look later. Re: 3. Multiple mappables with one colorbar: |
I took a look, and the norm now seems to connect automatically, but you still need to manually connect the images before you swap out the colormap. If we made a 'Mappable' class, we could definitely simplify this, by having a single object that holds the norm+colormap for all images in the example above. I imagine this class would then handle everything that deals with the data→norm→colormap transform, i.e. I think you are right that if we want to implement this, it would be a good idea to bundle it with this PR. Personally I would suggest nac = mpl.cm.NormAndColor(norm='LogNorm', cmap='Viridis')
ax[0].imshow(A, norm=nac)
ax[1].imshow(B, norm=nac) If this rhymes with what you are thinking, I'll take a look at implementing it. |
Maybe the usecase in #19515 (by @greglucas ) of choosing vmin and vmax, which I've often done that based on histograms so a linked zoom on histogram and update vmin and vmax w/ the corresponding values. Also just to make sure I'm on the same page, I'm interpreting the figures above as: And now I'm wondering where one of the use cases I was originally thinking of fits: ETA: nvariate is also weird cause it could maybe work w/ scalermappables and the hard part is it knowing how to input match. And yes these all have the same top level signature: |
@story645 The PR as it stands works like this: as you say it does not support:
When I was talking about a coupled norm, I was thinking of norms that take two arguments, i.e. If we want to introduce norms that take two arguments, we definitely need a EDIT: I'll take a look at implementing a |
I took a crack at implementing a import matplotlib
matplotlib.use('qtagg')
import matplotlib.pyplot as plt
import numpy as np
np.random.seed(19680801)
data = np.linspace(0.1, 0.6, 6)[:, np.newaxis, np.newaxis] * np.random.rand(6, 10, 20)
nac = matplotlib.cm.NormAndColor()
nac.vmin = np.min(data)
nac.vmax = np.max(data)
fig, axs = plt.subplots(2, 3)
fig.suptitle('Multiple images')
for i, ax in enumerate(axs.ravel()):
im = ax.imshow(data[i], norm = nac)
ax.label_outer()
cb = fig.colorbar(im, ax=axs, orientation='horizontal', fraction=.1)
plt.show() The video shows how the images couple: multiple_images.mp4(Additionally, in https://matplotlib.org/stable/gallery/images_contours_and_fields/multi_image.html, changing the colormap on the colorbar does not propagate to the images, but these changes propagate in the video above) This implementation also allows us to couple ND colormaps and 1D colormaps, so that changes in one propagates to the others: import matplotlib
matplotlib.use('qtagg')
import matplotlib.pyplot as plt
import numpy as np
np.random.seed(19680801)
data = np.random.rand(2, 10, 20)
fig, axes = plt.subplots(1, 3, figsize=(10,3))
im_2D = axes[0].imshow(data, cmap='BiOrangeBlue')
im_0 = axes[1].imshow(data[0], norm=im_2D.nac[0])
im_1 = axes[2].imshow(data[1], norm=im_2D.nac[1])
cb = fig.colorbar(im_0)
cb = fig.colorbar(im_1)
for ax in axes:
ax.label_outer()
plt.show() coupled_bivar.mp4@timhoffm Is this what you had in mind? I had to build this in a branch further down the pipeline than this PR, because it needs some features of the plotting functions not available in this PR. If you want to take a look, the |
Creation and tests for classes containing multivariate and bivariate colormaps.
Adds support for __getitem__ on colors.BivarColormap, i.e.: BivarColormap[0] and BivarColormap[1], which returns (1D) Colormap objects along the selected axes
removal of ColormapBase Addition of _repr_png_() for MultivarColormap addition of _get_rgba_and_mask() for Colormap to clean up __call__()
Also adds a an improvement to colors.BIvarColormap.__getitem__() so that this returns a ListedColormap object instead of a Colormap object
Also removed all set_ functions, and replaced them with with_extremes()
As part of your transition plan, strongly suggest sketching out what the new version of the docs are going to look like. 99.99% of folks don't want multiple data sets, norms, or colormaps so how are we going to convey this complexity without making our docs unreadable? The idea of a single norm is already hard to get across and a source of confusion. |
Thsnjs for the reminder on docs. I propose to think of docs as soon as we have an idea how the architecture could look like. That said, I‘m not overly worried about docs. They won’t determine whether we can support multi-valued coloring or not. If we can come up with a reasonable architecture and API, I expect that the docs will fall in place (not to say that we should take docs lightly, but the architecture structure will guide the docs). To quote the zen: If the implementation is hard to explain, it's a bad idea. And following: If the implementation is easy to explain, it‘s easy to document. |
Definitely, and @trygvrad has written a couple of GSOC blog posts that could seed new user guide entries:
The plan is to write standalone user guide pages/tutorials/examples and cross reference rather than trying to weave this through what we already have. One of the advantages of pipeline approach is that the signature (and therefore signature docs) more or less stay the same, w/ the heavy lifting on describing what's going on being in the API docs for the specific objects and in multivariate/bivariate specific usage docs. |
So we have discussed how this should come in at the weekly GSOC meetings, and also at the Matplotlib weekly meetings, but I see that it has not been clearly communicated via text. As a reminder: the scope of this has changed with @timhoffm s suggestions, but the original plan looked something like this:
This brings us to a state where we can include this in a release (labeling the new part of the API as provisional).
It is imperative that we divide the functionality into separate PRs, so that we can discuss different implementation details in isolation. We cannot have a situation where we discuss the internal pipeline in the same thread as colormaps and api. To some extent we can discuss all of these things at the same time, but we cannot do it in the same place. Furthermore, if we can keep the discussions sequential, we greatly reduce the workload. It is of no great importance to me if this is developed on main or in a feature branch.
@jklymak
It is not quite clear to me what part of the docs you want to see. There are code blocks showing how the new API is intended to be used at the top of this post, but if you want to see how this looks in more detail there are some prototype examples for the docs here and here with colormap references here and here. In addition there are the blog posts @story645 linked to previously. As for the docstrings, I am imagining minimal changes, i.e. for the norm_doc:
changes to
But again, the intention was to avoid a discussion of this here, and save these details for the a different PR (or at least until we have a prototype of the [Note that with the changes @timhoffm are suggesting above, we have to merge PR 2 and 3 in the list above into a single PR. This is not ideal from a PR-review point of view, and this is something I will discuss with @story645 @ksunden at the weekly GSOC meetings, unless the circumstances change at the Matplotlib weekly meeting tomorrow.] |
I very much second this concern and am glad that @trygvrad raised it. I'm supportive of doing the work on a feature branch as a way to avoid having partial functionality merged into main, but only if it's mostly a step in managing the release - meaning that the release manager (likely @ksunden or @QuLogic) would be handling this and discussion would be limited to merge specific topics. I fully agree w/ @trygvrad that discussions on design and implementation should be happening on the PRs merging into the feature-branch and should be scoped to the specific PRs and more general concerns should be raised at the meeting/mailing-list/discourse. I also very much agree with the concern of every decision being relitigated every time as that will effectively stop work as much as any hard block - ultimately this is coming in as a provisional pipeline that we are likely to leave provisional for years (I think it took us 2+ to make subplot mosaic non-provisional) so there will be room to change and refactor, and - as I have been told many times - that will be easier to do with something in place. |
May a weakref registry on the norm/mapper so that on autoscale the norm can go consult everyone who is using it? Can probably be done independent of this work though.
Based on our experience with getting mpl 2.0 out the door, I recommend against this 🤣 I have not fully caught up on the discussion (or the code), but I am still 👍🏻 on getting this in across a couple of PRs. The assumption in doing it that way is that you can split it up nicely, which assumes you understand your interfaces, which sometimes you do not actually understand until you have built the whole thing so it is not risk-free. However, one mega PR (or a long-lived development branch) has its own risks (hard to review, need to rebase, all-or-nothing) and on net I think trying 4-5 PRs is the better option. |
Thats fine - just the amount of workshopping in this PR gave me pause. And unlike something like subplot_mosaic, this hits much of the core functionality of the library. |
That's understandable and also this hitting so much of the core is why there's so much workshopping in the PR - we're trying to get to a rough consensus on the requirements/constraints/interface on this new pipeline so that @trygvrad can move forward on implementation such that the review of the implementation PRs can focus on:
|
I pushed some updates to this branch, but this is largely just because I want to see the code coverage, and this is the easiest way for me to see how the code coverage changes. Provisional names are |
We discussed this in the Matplotlib weekly meeting today, and we will split this into 2 PRs:
I'll make a comment here once 1. is ready. |
f9b2bf0
to
e271b85
Compare
VectorMappable with data type objects Creation of the VectorMappable class correctetion to _ensure_multivariate_norm() since colors.Normalize does not have a copy() function, we cannot easily convert a single norm to a sequence of norms. better use of union for colormap types Defines ColorMapType as Union[colors.Colormap, colors.BivarColormap, colors.MultivarColormap] in typing.py Suport for multivariate colormaps in imshow, pcolor and pcolormesh safe_masked_invalid() for data types with multiple fields NormAndColor class cleanup of _ImageBase._make_image() Rename To Mapper and reorganizatino changed name of new class from Mapper to Colorizer and removed calls to old api changed name of new class from Mapper to Colorizer Mapper → Colorizer ScalarMappableShim → ColorizerShim ColorableArtist → ColorizingArtist also removed all internal calls using the ColorizerShim api
e271b85
to
b4af71e
Compare
4a87c8b
to
c5b1d06
Compare
Creation of the VectorMappable class
**NOTE: This PR builds upon the BivarColormap and MultivarColormap classes introduced in ** #28454
PR summary
This PR has been in development during 2024 GSOC as a responese to #14168 Feature request: Bivariate colormapping.
This requires the introduction of multiple new classes, and changes to different parts of the codebase. It has therefore been suggested to introduce this functionality over multiple PRs, so that the changes are easier to review this is the 1st of 3(?) PRs that will resolve #14168.
The end goal of this is to allow for the following funcitonality:
The current class hierarchy can be visualized as follows:

This PR proposes to replace ScalarMappable with a new class VectorMappable, so that multivariate functionality can be inherited by all the relevant plotting methods, as shows below. This includes support for multiple norms (the private variable

._norm
always stores a list), support for multiple arguments to .set_vmin(), .set_vmax(), changes to .to_rgba() to support bivariate and multivariate colormaps, as well as support functions to parse multivariate data (top level functions in cm.py).This PR does not contain the changes needed in the plotting functions and docs. These will follow a separate PR, as outlined in the figure above. (a (slightly outdated) full implementation is available in the branch here)
PR checklist
¹This is the 2nd out of a series of PRs, and cannot by itself close #14168. Most features are tested, but some private functions do not have complete tests, as they are better tested in a later PR where the private API is tested via the publicly available API (colors._ensure_multivariate_params() etc.)