The Wayback Machine - https://web.archive.org/web/20210918015749/https://github.com/matplotlib/matplotlib/pull/19515
Skip to content
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

Colorbar axis zoom and pan #19515

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

@greglucas
Copy link
Contributor

@greglucas greglucas commented Feb 15, 2021

PR Summary

I've wanted to zoom/pan on values rather than extents of the image before and haven't figured out a clean way to do that without adding widgets. For example, if I set the range poorly the first time with my data and want to zoom in on a specific region of data, or if outliers made the vmin/vmax too large the first time. This adds zoom and pan capabilities to the colorbar. When zooming and panning on the axis, this updates the vmin/vmax of the scalar mappable norm associated with the colorbar. Currently, this changes the xlim/ylim of the axis holding the colorbar, which I don't think is desired behavior.

ezgif-6-88618ea58e72

For this to work, we need the parent axis for events, and the scalar mappable object. I wasn't sure where the best place to implement these methods is, as we need to override the parent axis event handlers, but that doesn't have access to the scalarmappable... I might be overlooking something obvious here too, so better suggestions welcome
.

Interactive example if you use the zoom/pan on the colorbar axis:

import matplotlib as mpl
import matplotlib.pyplot as plt
import numpy as np


rand_data = np.random.random((10, 10))*100

fig, [ax1, ax2, ax3] = plt.subplots(ncols=3)
cm = mpl.cm.get_cmap('plasma')
norm = mpl.colors.LogNorm(1, 100)
sm = mpl.cm.ScalarMappable(norm=norm, cmap=cm)

mesh1 = ax1.pcolor(rand_data, cmap=cm, norm=norm)
mesh2 = ax2.pcolorfast(rand_data, cmap=cm, norm=norm)
mesh3 = ax3.pcolormesh(rand_data, cmap=cm, norm=norm,)

cb3 = fig.colorbar(sm, ax=[ax1, ax2, ax3],
                   extend='max', orientation='horizontal')

plt.show()

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
@anntzer
Copy link
Contributor

@anntzer anntzer commented Feb 15, 2021

Fwiw the feature looks pretty natural to me, and it's certainly something I've been wanting to have before.

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented May 26, 2021

Now that #20054 is in I think this is now working properly on horizontal/vertical colorbars.

I didn't find any tests of the Zoom/Pan tools, am I missing those somewhere, or do we not test those ones and only test the widgets separately?

@greglucas greglucas changed the title [WIP] Colorbar axis zoom and pan Colorbar axis zoom and pan May 26, 2021
@greglucas greglucas marked this pull request as ready for review May 26, 2021
Copy link
Contributor

@jklymak jklymak left a comment

OK, so if I zoom, I change the color limits of the ScalarMappable, but if I set the limits of the axis, i.e. with set_xlim it zooms in on part of the colorbar (at least after #20054) but leaves the mappable unchanged. This is inconsistent, and just to be clear the new behaviour in #20054 was requested for various reasons I dont' fully understand.

Further, I don't see how to expand the limits with this GUI API.

Overall I'm not 100% sold that the colorbar is the right GUI for this.

I wonder if slightly more obscure, but equally evocative mouse motions could be used. ie. perhaps shift-scroll-up/down to increase the range and ctrl-scroll-up/down to move the centre? Then its not confused with just setting the limits?

Anyway, I think this is an interesting idea, but I'm not sure if it should be exactly the same as zoom and pan from the toolbar.

@timhoffm
Copy link
Member

@timhoffm timhoffm commented May 27, 2021

I agree with @jklymak concerns. We have two spaces, the data space (a, b) and the normalized "color" range (0, 1), which is associated with the color values. A the colored area in a colormap represents this color range and naively, zooming-in into the color range would result in a subrange and this a fraction of the color-range to be used, while the norm (i.e. the data-to-color relation) is unchanged. This however is less useful because you don't get additional detail. Actually, I'm wondering if it is useful at all.

What people usually want is to manipulate the norm. Technically, you would like to move the ticks on the colorbar, but that's not something we can directly support as a mouse interaction.

So two questions:

  1. Is the naive color-range manipulation something that we consider supporting at all? If so, we cannot use zooming and panning tools for norm manipulation.
  2. If not, is the changed semantics of the tools for colorbars as proposed here bearable in terms of user expectation or is that too confusing?

Mouse scrolling

There are standard conventions for mouse scrolling

scroll-up/down: vertical scroll
shift+scroll-up/down: horizontal scroll
ctrl+scroll-up/down: zoom in/out

For a regular axes the first to should translate to pan and the third should translate to zoom. (We don't have this implemented right now, but it would be a nice addition if anybody is interested in picking this up.)

We'd have to think if these could be resaonbly be adapted for colorbars in the above context.

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented May 27, 2021

Maybe a use-case for why I wanted this in the first place would be helpful. When making figures, coauthors will often say, well what if vmin/vmax were just a little higher/lower, how would that look? That could be done with adding widgets/sliders to control the vmin/vmax, but I don't want to be adding components, so I figured this was a good way to fit it in. Zoom/pan seemed natural to me, but if it isn't to other people we can consider some other interactions as well.

  • set_xlim() - interesting, I hadn't considered that. It seems like set_xlim/set_ylim on a Colorbar axis should be dispatched to norm.vmin/norm.vmax and not zoomed in to a smaller data range. For example, that could be misleading if you have extensions on your colorbar, but the limits at the top aren't actually the norm limits...

  • Zoom out / extend limits - This should be right click and drag I believe, the same as a normal axis because I just took most of the code from there. This also works with the home, back, and forward buttons too.

  • Moving the ticks on the colorbar - I'm not sure I follow here? The ticks are moving in the above example. Do you mean you want to interact with just the ticks outside the axis and not inside the axis where the colors are?

As for the questions about whether this seems bearable for user expectations or not... I came up with this idea, so I'd say yes, but YMMV :) Another thing here is that you have to explicitly enter zoom/pan mode to get this available, so it isn't something that will just blindly be available for someone mousing over the image, so there is that safety net in a sense.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 27, 2021

So maybe we start with set_xlim/set_ylim? Currently we do not have a colorbar.set_lim because we usually expect the user to change the colorbar by changing the vmin/vmax on the Norm. Hence the colorbar is a subordinate to the Norm. This PR inverts that and makes the colorbar actively control the norm. I will say this is also a confusion I see on stackoverflow that folks think the colorbar should somehow control the colors in the image. Currently, it is really easy currently to say "no it doesn't", the colorbar passively reflects the norm. If we no longer make that the case, then we need to think this through very carefully what the back and forth is and have thorough tests.

Again, maybe this is OK, but can we develop a programatic API first that doesn't rely on pan and zoom? i.e. colorbar.set_lim or colorbar.set_vlims, or?? That would make me a lot more comfortable rather than jumping to a GUI-only API, and would make the discussion a lot easier...

Some questions. If I set the vlim on the colorbar, and then I set it on the mappable, which is used at draw time? What if someone flips the limits? Norms often have other parameters than limits, in particulate everyone's favourite is symlognorm, or centered norm. How will this interact with those? Will the center move as well?

I'll put on the dev call for today. Feel free to stop by and pitch it. I don't know if we should discuss all the nitty gritty, but putting the PR on folks' radar and asking for comment here (or if you make a simpler colorbar.set_vlims PR) would be great.

@tacaswell tacaswell added this to the v3.5.0 milestone May 27, 2021
@QuLogic
Copy link
Member

@QuLogic QuLogic commented May 27, 2021

I didn't find any tests of the Zoom/Pan tools, am I missing those somewhere, or do we not test those ones and only test the widgets separately?

Zoom is tested in lib/matplotlib/tests/test_backend_bases.py::test_interactive_zoom.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 27, 2021

I guess the other UI for this is instead of mucking with the colorbar directly, a norm editor pops up in a separate helper window. This could also change the colormap.

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented May 27, 2021

@jklymak, you missed a good lengthy discussion today :) I think what you're proposing is essentially @anntzer's project here: https://github.com/anntzer/mplinorm

I'll be giving this some more thought over the next week and try to improve the UI.

@tacaswell
Copy link
Member

@tacaswell tacaswell commented May 28, 2021

OK, so if I zoom, I change the color limits of the ScalarMappable, but if I set the limits of the axis, i.e. with set_xlim it zooms in on part of the colorbar (at least after #20054) but leaves the mappable unchanged.

I think this is a mistake, will open a different issue about that next, but resolving that is probably a pre-req for merging this.

Further, I don't see how to expand the limits with this GUI API.

I would assume that right click zoom and right-click pan "zoom out" like they do with normal axes?


I think that the pan/zoom verbiage is semantically correct (if a bit jarring on first pass), but the UI needs a bit of tuning to make it clearer what is happening.

Although we implement the colorbar on an (increasingly standard) Axes, it is much more like the tick labels than it is like an normal "image". Under the hood we have transforms that go from data space -> x/y. The absolute position within the figure is meaningless, but by putting ticks on the Axes we can locally give meaning to the (relative) position. Even if we remove the ticks, we the relative positions with in an Axes still have meaning. When we adjust these transforms (by adjusting the x and y limits) we call this "panning" and "zooming" (depending on if we change the dynamic range or not).

Similarly, when we color map we have a transform (implemented in 2 parts: the norm and the colormap) that take data from dataspace -> RGB. The absolute color has no meaning (change the color map does not change the inherent meaning of the data any more than translating the whole axes around the figure changes the meaning!), but the relative colors tell us something about the relations between the data. If we then add a colorbar then we can get back access to the absolute values the same way ticks do.

In the images below, I think it is un-controversial to say "the y-axis was zoomed" or "the y-axis was panned". Doing exactly the same thing to the clim I think makes sense to use the same wording (there are some slight differences like when the curve goes out of the bounds it gets clipped but the color mapping saturates, could set over/under colors to transparent to get the same effect). Given the limitations of screens (we can not get a color axis sticking out of the screen!), I think implementing pan/zoom on the colorbar to "pan" and "zoom" in norm-space is the next best thing.

zoom
pan

Please forgive the copy-pasta nature of this code!

import matplotlib.pyplot as plt
import numpy as np

th = np.linspace(start := 0, stop := 2 * np.pi, N := 1024)
data1d = np.sin(th)

data2d = np.sin(th[:, np.newaxis]) * np.cos(th[np.newaxis, :])

curve_axes = ["1d", "y zoom in", "y zoom out"]
image_axes = ["2d", "color zoom in", "color zoom out"]

fig, ax_dict = plt.subplot_mosaic(
    [
        curve_axes,
        image_axes,
    ],
    constrained_layout=True,
)

for an in curve_axes:
    ax = ax_dict[an]
    ax.plot(th, data1d)
    ax.set_title(an)
    if an == "y zoom in":
        ax.set_ylim(-0.01, 0.01)
    elif an == "y zoom out":
        ax.set_ylim(-100, 100)
    else:
        ax.set_ylim(-1, 1)

for an in image_axes:
    ax = ax_dict[an]
    im = ax.imshow(
        data2d,
        extent=[start - (stop - start) / (2 * N), stop + (stop - start) / (2 * N)] * 2,
    )
    ax.set_title(an)
    fig.colorbar(im, ax=ax, aspect=7)
    if an == "color zoom in":
        im.set_clim(-0.01, 0.01)
    elif an == "color zoom out":
        im.set_clim(-100, 100)
    else:
        im.set_clim(-1, 1)

fig.savefig("/tmp/zoom.png")


curve_axes = ["1d", "y pan up", "y pan down"]
image_axes = ["2d", "color pan up", "color pan down"]

fig, ax_dict = plt.subplot_mosaic(
    [
        curve_axes,
        image_axes,
    ],
    constrained_layout=True,
)

for an in curve_axes:
    ax = ax_dict[an]
    ax.plot(th, data1d)
    ax.set_title(an)
    if an == "y pan up":
        ax.set_ylim(0, 2)
    elif an == "y pan down":
        ax.set_ylim(-2, 0)
    else:
        ax.set_ylim(-1, 1)

for an in image_axes:
    ax = ax_dict[an]
    im = ax.imshow(
        data2d,
        extent=[start - (stop - start) / (2 * N), stop + (stop - start) / (2 * N)] * 2,
    )
    ax.set_title(an)
    fig.colorbar(im, ax=ax, aspect=7)
    if an == "color pan up":
        im.set_clim(0, 2)
    elif an == "color pan down":
        im.set_clim(-2, 0)
    else:
        im.set_clim(-1, 1)
        
fig.savefig("/tmp/pan.png")

plt.show()

As mentioned on the call, this only makes sense for continuous (i.e. not categorical) norms.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 28, 2021

I don't have any problem with the semantics of zooming. I have practical concerns about the complexity of two-way linking of the colorbar to the mappable/norm it represents.

I do have another issue with the GUI aspect of zooming on a colorbar. @greglucas has a nice hefty colorbar up there, and you can still see him struggle to get the drag box to stay in the colorbar. I almost never have such large colorbars as I consider them a waste of space so I would be even more screwed. Below is a published example:

colorbars

@anntzer
Copy link
Contributor

@anntzer anntzer commented May 28, 2021

struggle to get the drag box to stay in the colorbar.

I don't think you need to keep the cursor in the colorbar; as long as the initial click is in it, the drag box will just be clipped to the axes.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 28, 2021

Thats good - but it's still a little fiddly...

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented May 28, 2021

@jklymak, the colorbar and mappable are currently directly linked. cb.norm is cb.mappable.norm So, if you update the vmin/vmax of either the colorbar or the image mappable, that state should get propagated. I think this is the way it should be as well, I want my colorbar to represent the data in my image when I've updated the state and not lag behind or be out of sync somehow.

I suppose if you make your colorbar too small and can't interact with it, then that just means you can't take advantage of this feature as easily, it doesn't adversely affect anything. There was a discussion about trying to interact with the ticks instead of the axis/colored area to make it more explicit as well what we are changing.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 28, 2021

@jklymak, the colorbar and mappable are currently directly linked. cb.norm is cb.mappable.norm So, if you update the vmin/vmax of either the colorbar or the image mappable, that state should get propagated. I think this is the way it should be as well, I want my colorbar to represent the data in my image when I've updated the state and not lag behind or be out of sync somehow.

Sure. But that does not necessarily have anything to do with the axes min/max. You are proposing that it does, and @tacaswell is proposing that zooming in on the colorbar is a "mistake" for some reason I've not heard. I'm not 100% against either proposal, but it needs some discussion. And just to reiterate, folks have asked to be able to zoom in on the colors on the colorbar, but to preserve the norm/colormap mapping. The ability to do so is new in #20054, so now is definitely the time to discuss if we don't want that behaviour, but except to implement these semantics, I'm not sure what the argument is against it.

I still feel the way forward here is if we are going to make this an action on the colorbar, we should first define the colorbar method that will do this, and leave the GUI discussion out of it.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 28, 2021

Just a note about the limits corresponding to vmin/vmax. This is in the comments, which seems to indicate that a larger colorbar axes than just vmin and vmax is explicitly allowed in the old API (imagine contours on a plot, saturated at high values, but you still want them labeled on the colorbar.

        # copy the norm and change the vmin and vmax to the vmin and
        # vmax of the colorbar, not the norm.  This allows the situation
        # where the colormap has a narrower range than the colorbar, to
        # accommodate extra contours:

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented May 28, 2021

I guess I still just don't quite understand this use case and wonder if it can't be solved in a different manner... Is the real request to be able to use less of the colormap than is available by default because you want it to saturate at a different value? I've used this crude hack in the past to change the saturation point, so maybe a public function on a colormap to do something similar would be the better place for it than messing with limits on an axes?

cmap = mpl.cm.get_cmap('inferno')
# Remove the bottom 10% of the cmap
cmap_small = mpl.colors.LinearSegmentedColormap.from_list('inferno_small', cmap(np.linspace(0.1, 1.0, 256)))

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 28, 2021

pc = ax.contour(np.arange(1, 11), np.arange(1, 11), np.arange(100).reshape(10, 10), 
                levels=np.arange(0, 100, 10), vmin=0, vmax=50)
cb = fig.colorbar(pc)

is a use case where the norm goes from 0 to 50 but the colorbar goes to 100.

@jklymak
Copy link
Contributor

@jklymak jklymak commented May 29, 2021

BTW, just to be clear, I'm not saying the above functionality should trump the proposed functionality, just that it is something we will break...

@tacaswell
Copy link
Member

@tacaswell tacaswell commented Jun 3, 2021

I was mostly thinking about the case where you make the range of the color bar narrow than the vmin/vmax which seems like a miss-feature to me (because then we have put colors in the figure that do not have a key to indicate what that color means), but making the top/bottom of the color bar bigger than the norm (and basically implementing the extend arrows by having a constant area in the color bar).

I think I am going to back away from "the color bar limits should always exactly match the norm limits" to "the norm limits should be contained in the color bar limits".

The first thing that occurred to me under that framing is we have a norm_tracks_limits setting on the color bar. If True we get the behavior with pan/zoom that @greglucas is proposing here, if it is False, then when the user sets the limits on the color bar we expand them to include vmin/vmax (there is precedent for this with the nonsingular step) and if they are bigger just let it happen. In all of these cases the "extend" arrows will still be consistent.

I think the constraints are:

  • every possible color in the cmap should be shown with a correct value in the color bar
  • ever tick / value along the axis of the color bar should be matched with the color that value would be in the data

@jklymak
Copy link
Contributor

@jklymak jklymak commented Jun 3, 2021

every possible color in the cmap should be shown with a correct value in the color bar

That seems overly prescriptive. Folks have specifically asked to have the limits tighter than vmin and vmax. Why would we forbid that?

@jklymak jklymak marked this pull request as draft Jun 17, 2021
@jklymak
Copy link
Contributor

@jklymak jklymak commented Jun 17, 2021

My understanding is that @greglucas was hoping to come up with a different UI here, so putting in draft

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented Jun 17, 2021

I did take a look at trying to create a better UI, but ran into some issues that I'll have to look at in more detail. I'll put some notes here primarily for myself so I don't forget later. But, this is definitely still in a draft state :)

  1. The Zoom box is drawn as a rubberband in backend-specific implementations. So, if I wanted to override that, I may have to do it on each backend. Alternatively, I could possibly shortcut out early if it is a ColorbarAxes and add two ax.vlines() (or a tickline-like extension) where the positions get updated as you drag and then removed once Zoom is off. The second suggestion is complicated by 2 below.
  2. The interaction currently takes place on the outer_ax, which means it doesn't know it is a Colorbar, so we would need to add a private attribute identifier for this. However, then we don't want to do the draw on the outer_ax, but rather the inner_ax, so really I need to look at how to make all of the interaction take place on the base ColorbarAxes and dispatch the methods properly from the base class instead.

@richardsheridan
Copy link
Contributor

@richardsheridan richardsheridan commented Jun 17, 2021

As mentioned in the dev call, I achieved something like this in my project using just the existing colorbar API and the picker functionality. The relevant functions are below:

https://github.com/richardsheridan/magic-afm/blob/c39b6e135263cc8d9b30b7e20fd8c1e9416257ae/magic_afm/gui.py#L1395-L1419

customize_colorbar is triggered in response to pick events on the canvas and (among other customizations) sets the clim directly on the underlying solid. This allows a user to simply click where they would like the top and bottom ends of their colorbar range.

expand_colorbar essentially only calls the latter part of Colorbar.update_normal() so that the actual gradient part of the colorbar is allowed to "expand" to the full range of the axes. This is a helpful mode to have available if the desired range of the image is much less than the total dynamic range of the image.

My point being, I think we can already produce functionality that covers the use case of the present PR with existing APIs, which -- if true -- would be a strong argument against invasive changes. My solution is not perfectly polished, as the ticks have some slightly weird but not terrible behavior by default. I will try to post a PR with a recipe suitable for discussion and review this weekend, hopefully to be converted into an example for the gallery.

@jklymak
Copy link
Contributor

@jklymak jklymak commented Jun 17, 2021

@richardsheridan I think you'll find some of the tick issues etc are ameliorated by the recent changes to colorbar, but maybe not?

@richardsheridan
Copy link
Contributor

@richardsheridan richardsheridan commented Jun 17, 2021

I think you'll find some of the tick issues etc are ameliorated by the recent changes to colorbar, but maybe not?

I pinned my app's dependency versions a while back but I'll try out the latest commits sometime soon since you bring it up

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented Jun 18, 2021

@richardsheridan, that sounds like another good possible solution! I had another meeting during the call today, but I should be able to make next week's dev call and I'd enjoy seeing a demo of that in action.

To me it still seems like something along these lines of "interactive manipulation of colorbar range" is desirable and now we need to figure out the best strategy of how to do it within MPL. I personally want it to be more than just an example of how to do it, I'm looking for this to be a keypress/event away within the GUI so I don't have to write my own click handler and attach it. I can already add a widget range slider to get a similar effect, but that doesn't help if I'm just doing quick-look plots.

@greglucas greglucas force-pushed the colorbar-axes-zoom branch 2 times, most recently from 148f157 to da898e6 Jul 9, 2021
@greglucas
Copy link
Contributor Author

@greglucas greglucas commented Jul 9, 2021

Now that #20501 is in, I came back to this and it is simpler now to just have to interact on one main axes.

Short summary of updates:

  • Removed interactivity from Categorical norms and contour(f)
  • Added tests for horizontal/vertical, zoom-in/zoom-out/pan
  • Refactored out some common helper methods to get axes bounds so the logic isn't duplicated here
  • Extended the rubberband selector on the zoom tool all the way to the horizontal/vertical bounds

For the UI/UX, I decided to try out the simple just extend the rubberband (drawn by the backends already with the zoom tool) all the way out to the edges of the rectangle in one direction, and I think that looks pretty good. The axvline/axhline idea would complicate things a bit by having to keep track of those artists and remove them after the zoom release. This keeps it consistent with the core "zoom" rubberband implementation on each backend.

@greglucas greglucas marked this pull request as ready for review Jul 31, 2021
@greglucas
Copy link
Contributor Author

@greglucas greglucas commented Jul 31, 2021

I'm marking this as ready for review again. I updated the gif in the original post showing the rubberband fully extended along the axis while zooming and that contour(f) colorbars not having this capability.

@jklymak jklymak added this to Medium Priority in v3.5 Milestone Aug 5, 2021
Copy link
Contributor

@jklymak jklymak left a comment

Overall this seems to work, just a couple of small questions....

lib/matplotlib/axes/_base.py Outdated Show resolved Hide resolved
@@ -488,6 +488,16 @@ def __init__(self, ax, mappable=None, *, cmap=None,
if isinstance(mappable, contour.ContourSet) and not mappable.filled:
self.add_lines(mappable)

# Link the Axes and Colorbar for interactive use
self.ax._colorbar = self
Copy link
Contributor

@jklymak jklymak Aug 10, 2021

There is already a ax._colorbar_info that should identify an axes as a colorbar.

Copy link
Contributor Author

@greglucas greglucas Aug 10, 2021

I think the only thing I used this for is knowing the orientation, which isn't currently in the _colorbar_info. I could add that in there as another quantity... Would that be able to get out of sync though if someone passes their own axes into Colorbar, or is that considered robust to check if an axes is used for a colorbar? (It looks like currently someone can pass their own axes into matplotlib.colorbar.Colorbar(ax, orientation='horizontal') and ax wouldn't have _colorbarinfo unless it was previously created with the make_colorbar_axes functions?

Copy link
Contributor

@jklymak jklymak Aug 11, 2021

No you are right, thats not there is the user adds the axes.

I guess this is OK - given that this and recent PRs are passing info back and forth between the axes and the colorbar a fair bit I do wonder if there is a better way to e doing this. I appreciate your idea for making the colorbar a special type of axes now.

self.ax._colorbar = self
for x in ["_get_view", "_set_view", "_set_view_from_bbox",
"drag_pan", "start_pan", "end_pan"]:
setattr(self.ax, x, getattr(self, x))
Copy link
Contributor

@jklymak jklymak Aug 10, 2021

Why are we defining the methods on the colorbar object, and then duplicating them on the axes?

Copy link
Contributor Author

@greglucas greglucas Aug 10, 2021

These methods need to have access to the Colorbar's norm data (inside of them I grab self.norm.vmin), so they don't have access to that data if they live only on the axes.

Copy link
Contributor

@jklymak jklymak Aug 12, 2021

OK, so what happens now if the user changes the xlim of the colorbar axis? It looks to me that nothing happens.... Is that what we want?

My concern here is that zoom on normal axes means "adjust the axes limits" (and populates a stack of previous limits). Here it does that, but set_xlim will behave differently.

Copy link
Contributor Author

@greglucas greglucas Aug 12, 2021

If someone calls set_xlim() right away, that is respected. However, once someone starts interacting with the colorbar, the norm expands to the limits of the axis. Initially, the norm does not follow the xlim. That behavior could be argued either way I think, so I'm not inclined to change it in this PR... If someone wants to type in set_xlim() after having interacted that will be respected here too I think.

My preference is to keep the behavior as-is, and if someone has an issue with the mismatch between set_xlim() and set_clim() then we cross that bridge in a follow-up PR.

Here is a quick example to test with:

Click for demo code
mport matplotlib as mpl
import matplotlib.pyplot as plt
import numpy as np
mpl.use('qt5agg')


rand_data = np.random.random((10, 10))*100

fig, [ax1, ax2, ax3] = plt.subplots(ncols=3)
cm = mpl.cm.get_cmap('plasma')
norm = mpl.colors.LogNorm(1, 100)
norm = mpl.colors.Normalize(1, 100)
sm = mpl.cm.ScalarMappable(norm=norm, cmap=cm)

mesh1 = ax1.imshow(rand_data, cmap=cm, norm=norm)
mesh2 = ax2.pcolorfast(rand_data, cmap=cm, norm=norm)
cb1 = fig.colorbar(sm, ax=[ax1, ax2],
                   extend='max', orientation='horizontal')
cb1.ax.set_xlim(0, 200)

pc = ax3.contourf(np.arange(1, 11), np.arange(1, 11), np.arange(100).reshape(10, 10),
                levels=np.arange(0, 100, 10)) #, vmin=0, vmax=50)
cb2 = fig.colorbar(pc, ax=ax3, orientation='vertical')


plt.show()

Copy link
Member

@QuLogic QuLogic Aug 21, 2021

Does this break Axes.cla?

Actually, I'm confused how these can work when they access self.<Colorbar stuff> when self should be an Axes that you've setattrd them on.

Copy link
Contributor Author

@greglucas greglucas Aug 21, 2021

The Axes methods are actually bound to the Colorbar object. Here is a print of colorbar.ax._get_view and colorbar._get_view showing they are both bound to the same object.

<bound method Colorbar._get_view of <matplotlib.colorbar.Colorbar object at 0x7fc7b8eb4b10>>
<bound method Colorbar._get_view of <matplotlib.colorbar.Colorbar object at 0x7fc7b8eb4b10>>

Does this break Axes.cla?

cb.ax.cla() does not remove these methods, so they are still there, but it does not impact anything other than interactivity, so I'm not sure how many people would be using cb.ax.cla() and expecting an original axes back and not doing a cb.remove() and creating a new one altogether... I'm not sure how we could replace the other methods, unless we kept them around somewhere just for that situation. I think this would be easier to fix properly if we had #20350 and could properly override these methods.

Copy link
Contributor Author

@greglucas greglucas Sep 16, 2021

It is definitely not very elegant, but I think I got it to work as expected in the latest commit. Note that even with this addition of replacing the old axes interactivity, there is still an incorrect extend triangle hanging around, so calling cla() on a colorbar axes has probably never truly worked as desired.

Here is a quick script to test that with:

import matplotlib as mpl
import matplotlib.pyplot as plt
import numpy as np

rand_data = np.random.random((10, 10))*100

fig, [ax1, ax2, ax3] = plt.subplots(ncols=3)
cm = mpl.cm.get_cmap('plasma')
norm = mpl.colors.LogNorm(1, 100)
norm = mpl.colors.Normalize(1, 100)
sm = mpl.cm.ScalarMappable(norm=norm, cmap=cm)

mesh1 = ax1.imshow(rand_data, cmap=cm, norm=norm)
mesh2 = ax2.pcolorfast(rand_data, cmap=cm, norm=norm)
cb1 = fig.colorbar(sm, ax=[ax1, ax2],
                   extend='max', orientation='horizontal')
# cb1.ax.cla()
# newcb = fig.colorbar(sm, cax=cb1.ax, extend='min', orientation='horizontal')

pc = ax3.contourf(np.arange(1, 11), np.arange(1, 11), np.arange(100).reshape(10, 10),
                levels=np.arange(0, 100, 10)) #, vmin=0, vmax=50)
cb2 = fig.colorbar(pc, ax=ax3, orientation='vertical')


plt.show()

@greglucas greglucas force-pushed the colorbar-axes-zoom branch from da898e6 to d758998 Aug 10, 2021
lib/matplotlib/axes/_base.py Outdated Show resolved Hide resolved
lib/matplotlib/backend_bases.py Show resolved Hide resolved
self.ax._colorbar = self
for x in ["_get_view", "_set_view", "_set_view_from_bbox",
"drag_pan", "start_pan", "end_pan"]:
setattr(self.ax, x, getattr(self, x))
Copy link
Member

@QuLogic QuLogic Aug 21, 2021

Does this break Axes.cla?

Actually, I'm confused how these can work when they access self.<Colorbar stuff> when self should be an Axes that you've setattrd them on.

greglucas added 4 commits Aug 21, 2021
The zoom and pan funcitons change the vmin/vmax of the norm
attached to the colorbar. The colorbar is rendered as an inset
axis, but the event handler is implemented on the parent axis.
This helps for subclasses in finding the zoom/pan locations by
not having to duplicate the code used to determine the x/y locations
of the zoom or pan.
Setting the zoom selector rectangle to the vertical/horizontal
limits of the colorbar, depending on the orientation.

Remove some mappable types from colorbar navigation. Certain mappables,
like categoricals and contours shouldn't be mapped by default due to
the limits of an axis carrying certain meaning. So, turn that off
for now and potentially revisit in the future.
Adding tests for vertical and horizontal placements, zoom in, zoom out,
and pan. Also verifying that a colorbar on a Contourset is not able
to be interacted with.
@greglucas greglucas force-pushed the colorbar-axes-zoom branch from d758998 to 94a1f59 Aug 21, 2021
@greglucas
Copy link
Contributor Author

@greglucas greglucas commented Sep 16, 2021

Just a friendly ping on this to see if there are any other comments/questions on this.

@greglucas
Copy link
Contributor Author

@greglucas greglucas commented Sep 16, 2021

As discussed on the call today, I updated the API release note and the final commit also adds a way to get cla() to remove the colorbar interactivity.

@greglucas greglucas force-pushed the colorbar-axes-zoom branch from 5032eeb to f35bf2c Sep 16, 2021
for x in self._orig_interactive_funcs:
setattr(self.ax, x, self._orig_interactive_funcs[x])
Copy link
Member

@QuLogic QuLogic Sep 17, 2021

I think you can just do delattr to reset back to the original class method.

Copy link
Contributor Author

@greglucas greglucas Sep 17, 2021

Yes, that was much simpler. Thanks!

This adds logic to remove the colorbar interactivity and replace
the axes it is drawn in with the original interactive routines.
@greglucas greglucas force-pushed the colorbar-axes-zoom branch from f35bf2c to 5322051 Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v3.5 Milestone
Medium Priority
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants