FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

FIX: colorbar placement in constrained layout by jklymak · Pull Request #11648 · matplotlib/matplotlib · GitHub

Repository navigation

FIX: colorbar placement in constrained layout - #11648

Merged
ImportanceOfBeingErnest merged 3 commits into
matplotlib:masterfrom
jklymak:fix-colorbars-constrained-layout
Oct 2, 2018
Merged

ImportanceOfBeingErnest merged 3 commits into
matplotlib:masterfrom
jklymak:fix-colorbars-constrained-layout

Conversation

jklymak commented Jul 12, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Closes #11641

Previously, while using constrained_layout putting a colorbar on a figure put the colorbar to the right (or other location) of the parent gridspec of the axes specified.

However, the non-constrained_layout way of doing this would just put to the right of the listed axes.

This PR makes the constrained_layout behaviour the same as non-constrained_layout

import numpy as np
import matplotlib.pyplot as plt

for CL in (True, False):
    mappable = []
    fig, axs = plt.subplots(2, 2, constrained_layout=CL, figsize=(4.5, 4))
    for nn, ax in enumerate(axs.flatten()):
        data = np.random.randn(10, 10)
        mappable += [ax.pcolormesh(data)]
    fig.colorbar(mappable[0], ax=axs[:, 0])
    fig.colorbar(mappable[1], ax=axs[0, 1])
    fig.colorbar(mappable[3], ax=axs[1, 1])
    fig.suptitle('constrainedlayout %s'%CL)
    plt.show()

Before:

Behaviour of where first colorbar went changed significantly under constrained_layout


After

Behaviour more consistent...


PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant

jklymak added the topic: geometry manager LayoutEngine, Constrained layout, Tight layout label Jul 12, 2018
jklymak added this to the v3.1 milestone Jul 12, 2018
jklymak force-pushed the fix-colorbars-constrained-layout branch from 6a70f48 to 8f20012 Compare July 12, 2018 21:31

jklymak commented Jul 13, 2018 •
edited
Loading

Copy link
Copy Markdown
Member Author

Marking WIP as I'd like to add a couple of examples.... Done

jklymak force-pushed the fix-colorbars-constrained-layout branch from 6a67460 to 0e482e3 Compare July 13, 2018 20:08
jklymak modified the milestones: v3.1, v3.0.x Sep 19, 2018

jklymak commented Sep 19, 2018

Copy link
Copy Markdown
Member Author

Re milestoning as a bug fix. The behaviour here is much better than in 3.0.0

cm = ['RdBu_r', 'viridis']
for nn in range(2):
for mm in range(2):
ax = axs[mm, nn]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe use row, col instead of mm, nn.

# ``constrained_layout=True``

fig, axs = plt.subplots(3, 3, constrained_layout=True)
for ax in axs.flatten():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I would use ax.flat. While flatten() reshapes the array, flat directly returns an iterator over the array, which is all we are needing here.

# For the `~.axes.Axes.pcolormesh` kwargs (``pc_kwargs``) we use a
# dictionary. Below we will assign one colorbar to a number of axes each
# containing a `~.cm.ScalarMappable`; specifying the norm and colormap
# ensuresthe colorbar is accurate for all the axes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

missing space

#
# ``constrained_layout`` usually adjusts the axes positions on each draw
# of the figure. If you want to get the spacing provided by
# ``constrained_layout`` but then not have it update, then do the initial

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

leave out the "then" before "do". This sentence has too much "then" 😄

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks - I took out the first "then" so it now reads:

# If you want to get the spacing provided by
# ``constrained_layout`` but not have it update, then do the initial
# draw and then call ``fig.set_constrained_layout(False)``.

jklymak force-pushed the fix-colorbars-constrained-layout branch from 0e482e3 to 6d201de Compare September 22, 2018 04:00

jklymak commented Sep 22, 2018

Copy link
Copy Markdown
Member Author

Thanks for the careful review @timhoffm

jklymak force-pushed the fix-colorbars-constrained-layout branch from 6d201de to c3a17ab Compare September 24, 2018 17:50
jklymak force-pushed the fix-colorbars-constrained-layout branch from c3a17ab to f4438fb Compare September 24, 2018 18:52

Copy link
Copy Markdown
Member

I only had a quick look through the code, too quick to give a green checkmark here already, but in principle this looks good and it's definitely correct to not let the two cases of contrained layout=True or False differ substantially.

Now this PR introduces a new "Placing colorbars" example, which is a great idea. If that is the place to look for how to place colorbars in matplotlib, it's not yet complete, since there are lots of other ways, mostly via the cax argument.

  • use axes created from fig.add_axes
  • use axes created via gridspec with unequal width_ratios
  • show use of fraction, anchor arguments
  • use axes created via inset_axes (not tested yet, but should be a nice way to get axes outside the plot without taking space from the original axes); similar to demo_colorbar_with_inset_locator., but with the new core function.
  • use Axes Divider; or at least link to existing demo_colorbar_with_axes_divider
  • (at least one of the examples should show how to create a colorbar which is of the same height as a fixed-aspect plot)

Seeing how long that list has become, it might as well be suitable as tutorial?
Also, this list is sure too long to be handled within this PR.

jklymak commented Sep 27, 2018

Copy link
Copy Markdown
Member Author

My proposal would be for this PR to go in, and then if someone wants to add a couple more examples to the new example file, that would be great. I'd love to see how anchor and panchor work - I've never played with those or seen them in the wild. They could sure be better documented than they are now.

ImportanceOfBeingErnest left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

After going through this in more detail, I tried to construct cases for which this would fail. Turns out I didn't find any. So I agree that this should go in to be able to move on.

jklymak commented Oct 2, 2018

Copy link
Copy Markdown
Member Author

Great - any of those cases you think would be useful as tests we can add. Certainly I could break it in the future!

jklymak deleted the fix-colorbars-constrained-layout branch October 2, 2018 21:47
jklymak added a commit that referenced this pull request Oct 3, 2018
…648-on-v3.0.x

Backport PR #11648 on branch v3.0.x (FIX: colorbar placement in constrained layout)

QuLogic commented Oct 3, 2018

Copy link
Copy Markdown
Member

The test added here seems to have broken on Travis master somehow.

QuLogic commented Oct 3, 2018

Copy link
Copy Markdown
Member

Actually, it was already broken when it was merged, but it's been 9 days since the PR build, so I'm not certain what might have affected it.

jklymak commented Oct 3, 2018

Copy link
Copy Markdown
Member Author

The test passes locally on master...

jklymak commented Oct 3, 2018

Copy link
Copy Markdown
Member Author

@QuLogic 3.1 tests are passing on Master. Was it 3.0.x tests that have trouble?

QuLogic commented Oct 3, 2018

Copy link
Copy Markdown
Member

Well, I'm not sure what happened; it seems to have magically fixed itself. It was definitely failing right after merging this PR and also after merging the fix from #12366 that otherwise broke things.

Copy link
Copy Markdown
Member

@jklymak Since you asked for special cases for potential tests:

The following produces a rather strange figure

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.gridspec import GridSpec

for CL in (True, False):
    mappable = []
    fig = plt.figure(constrained_layout=CL, figsize=(4.5, 4))
    gs = GridSpec(2,2, figure=fig)
    ax1 = fig.add_subplot(gs[0,0])
    ax2 = fig.add_subplot(gs[1,0])
    ax3 = fig.add_subplot(gs[1,1])
    for nn, ax in enumerate([ax1,ax2,ax3]):
        data = np.random.randn(10, 10)
        mappable += [ax.pcolormesh(data)]
    fig.colorbar(mappable[0], ax=[ax1,ax3])
    fig.suptitle('constrainedlayout %s'%CL)
    plt.show()

But constrainedlayout is perfectly capable of handling it:

In contrast, when you add another axes later on, it'll overlap, both with and without constrained layout.

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.gridspec import GridSpec

for CL in (True, False):
    mappable = []
    fig = plt.figure(constrained_layout=CL, figsize=(4.5, 4))
    gs = GridSpec(2,2, figure=fig)
    ax1 = fig.add_subplot(gs[0,0])
    ax2 = fig.add_subplot(gs[1,0])
    ax3 = fig.add_subplot(gs[1,1])
    for nn, ax in enumerate([ax1,ax2,ax3]):
        data = np.random.randn(10, 10)
        mappable += [ax.pcolormesh(data)]
    fig.colorbar(mappable[0], ax=[ax1,ax3])
    ax3.remove()
    ax4 = fig.add_subplot(gs[0,1])
    fig.suptitle('constrainedlayout %s'%CL)
    plt.show()

Not sure if any of those should be considered valid or useful though.

jklymak commented Oct 4, 2018

Copy link
Copy Markdown
Member Author

Those were brain teasers. I'm OK w/ the first case inconsistency. Colorbar steals space from the axes you list, and there is no way for the third axes to know space was stolen from the other two without a draw-time hook like ConstrainedLayout.

The second case is a bit more concerning; I'd have thought ideally CL would forget about the lost axes, and move the colorbar over to the left. But... that's a pretty hard pathological case, so I'm not going to worry about it, unless there is a good use case for it, in which case it may be worth the effort.

Thanks a lot for sending these! Great examples...

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: geometry manager LayoutEngine, Constrained layout, Tight layout

Projects

None yet

Development

Successfully merging this pull request may close these issues.

constrained_layout and colorbar for a subset of axes

5 participants


Back | FazBrowse Home | New Git URL