Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Colorbar grid position #18575

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

Merged
merged 18 commits into from
Oct 16, 2020
Merged

Conversation

shawnchenx6
Copy link
Contributor

@shawnchenx6 shawnchenx6 commented Sep 25, 2020

PR Summary

I made a little change so that now the anchor argument in make_axes_gridspec mimic the behavior of that in make_axes. Now the postion of colorbar is determined by anchor and shrink, should be very useful when shrink < 1.0

The effect is demostrated in the plot below. Now I can place the colorbar at different place with different shrinked size.

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

x = np.random.randn(100)
y = np.random.randn(100)
fig = plt.figure()
ax = fig.add_subplot()
mappable = ax.scatter(x,y, c=y)
ax.figure.colorbar(mappable, ax=ax, anchor=(0,0.3), shrink=0.5) 

A good figure

Before the change, we can achieve that using ax.figure.colorbar(mappable, ax=ax, anchor=(0,0.3), use_gridspec=False) , however, this will create a colorbar as a Axes not a Subplot, when I use subplots_ajust to adjust the border margins, I got the following plot.

fig = plt.figure()
ax = fig.add_subplot()
mappable = ax.scatter(x,y, c=y)
ax.figure.colorbar(mappable, ax=ax, anchor=(0,0.3), shrink=0.5, use_gridspec=False) 
ax.figure.subplots_adjust(bottom=0.1)

A bad figure

It seems the colorbar Axes is ignored and my plot is re-placed according to its panchor parameter that make_axes set. It bothers me when I have multiple plots and wanna adjust the plots border margins and wspace, hspace.

gridspec offers a better approach and is the default setting when creating a colorbar, however, I want the colorbar to be 0.5 size and placed at the bottom right, which is mot implemented.

So this is what I did basically.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@shawnchenx6 shawnchenx6 mentioned this pull request Sep 25, 2020
6 tasks
@tacaswell tacaswell added this to the v3.4.0 milestone Sep 25, 2020
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
@shawnchenx6
Copy link
Contributor Author

I have modify the gallery example for colorbar to demonstrate this new feature. please kindly help check if it is appropriate.
Also, Is there any guide on how and where to put what's new entry

@shawnchenx6
Copy link
Contributor Author

I have added different locations for colorbar in the test to ensure the code is correct. please check the code and also the plots if possible.

@jklymak
Copy link
Member

jklymak commented Sep 27, 2020

Closing and re-opneing. All the tests seem to have freaked out for some reason (which I don't think is due to this PR).

@jklymak jklymak closed this Sep 27, 2020
@jklymak jklymak reopened this Sep 27, 2020
@shawnchenx6
Copy link
Contributor Author

shawnchenx6 commented Sep 27, 2020

my observation is when I push the code the github, the tests all passed, but when I merged branch matplotlib/master into my branch, the tests fail.
The merge happened here b356530

@jklymak
Copy link
Member

jklymak commented Sep 28, 2020

Yeah, pytest broke. Lets do another hard cycle

@jklymak jklymak closed this Sep 28, 2020
@jklymak jklymak reopened this Sep 28, 2020
examples/color/colorbar_basics.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Show resolved Hide resolved

np.testing.assert_allclose(
[cy1, cy0],
[y1 * shrink + (1 - shrink) * p0, p0 * (1 - shrink) + y0 * shrink])
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand why the anchor point p0 should be affected by shrink.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p0 is not affected by shrink, cy1 is the top of colorbar axes, which can be calculated using p0 and shrink
cy1 = y1 * shrink + (1 - shrink) * p0
while p0 and shrink do not affect each other

Copy link
Member

Choose a reason for hiding this comment

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

I'm not saying they affect each other; I'm saying that shrink has an effect on p0 for cy0/cy1, and I don't know why. If p0 is an anchor, then it should not get shrunk.

Copy link
Member

Choose a reason for hiding this comment

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

I looked at this again, and anchor is the anchor point of the colorbar, while I was thinking it was the anchor on the parent Axes, but that's defined by panchor. So this is why I was confused by this calculation.

However, that means that p0 doesn't seem to be calculated right, as it is calculating an anchor position in the parent Axes space (using y0/y1), but using anchor_y, which was used for the colorbar anchor position.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the test function, I will take location=right as an example

demo

the x0 y0 x1 y1 for the plot axes is [0.125, 0.11, 0.745, 0.88] (rounded to 2 digits) , blue region
the cx0 cy0 cx1 cy1 for cbar is [0.78, 0.22, 0.80, 0.61] , orange region

the p0 is the absolute y postion (red dot) of colorbar anchor against the plot, while anchor-y is a relative colorbar postion against the parent axes,
thus p0-y0 = (y1-y0) * anchor_y.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed the y value for colorbar anchor is the relative postion against the plot, not the absolute y position. Correct me if I am wrong.

Copy link
Member

Choose a reason for hiding this comment

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

@ShawnChen1996 I think what would be convincing would be an example that shows clearly this version does close to the same as the version with use_gridspec=False, just to make sure the semantics are the same for anchor and panchor. For completeness it would be good if the were tested at more than one anchor along all four sides.

I know below we suggested not making an image test, but it would be helpful to see what the image looks like, even if it doesn't make it into the test. I guess in situations like this, I actually lean towards adding an image test since its hard to tell by looking at the numbers if things are working. But at least for review, it would be super helpful to see the image.

If the kwargs are being used the same as the old meaning of the kwargs, then we still have a documentation issue, but that shouldn't block this PR.

ShawnChen1996 and others added 2 commits October 2, 2020 23:13
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
ShawnChen1996 and others added 4 commits October 2, 2020 23:14
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved

np.testing.assert_allclose(
[cy1, cy0],
[y1 * shrink + (1 - shrink) * p0, p0 * (1 - shrink) + y0 * shrink])
Copy link
Member

Choose a reason for hiding this comment

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

I looked at this again, and anchor is the anchor point of the colorbar, while I was thinking it was the anchor on the parent Axes, but that's defined by panchor. So this is why I was confused by this calculation.

However, that means that p0 doesn't seem to be calculated right, as it is calculating an anchor position in the parent Axes space (using y0/y1), but using anchor_y, which was used for the colorbar anchor position.

QuLogic
QuLogic previously requested changes Oct 9, 2020
Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

I could be wrong because the existing documentation is so terse, but from some tests with the example, I cannot see how this does what anchor and panchor are documented to do.

@shawnchenx6
Copy link
Contributor Author

To be clear, this PR is to allow colorbar to be placed at different locations with customized shrinked size, as an attempt to mimic what the function make_axes does.

As for what anchor and panchor are documented to do, I may need extra info here to help me understand what else needs to be done.

@shawnchenx6
Copy link
Contributor Author

Also, the colorbar y postion (cy0 & cy1) generated using this PR and using make_axes are the same, meaning the anchor_y value is the relative y position against parent axes

@QuLogic
Copy link
Member

QuLogic commented Oct 10, 2020

anchor and panchor are documented here:

anchor : (float, float), optional
The anchor point of the colorbar axes.
Defaults to (0.0, 0.5) if vertical; (0.5, 1.0) if horizontal.
panchor : (float, float), or *False*, optional
The anchor point of the colorbar parent axes. If *False*, the parent
axes' anchor will be unchanged.
Defaults to (1.0, 0.5) if vertical; (0.5, 0.0) if horizontal.

It is very short, but the way I read it, anchor is an anchor within the colorbar Axes, and panchor is an anchor within the parent Axes. That is, panchor is positively correlated with the eventual colorbar position, and anchor is negatively correlated with the colorbar position.

If this PR works the same as use_gridspec=False, then we 'just' have a documentation problem. It's also possible I've misread what these mean as well.

@shawnchenx6
Copy link
Contributor Author

import numpy as np
import matplotlib.pyplot as plt


def anchored_cbar_position(use_gridspec=True):
    data = np.random.normal(loc=5, size=(30, 40))

    # 4 * 4
    # each row using one location
    # each col using one anchor position
    fig = plt.figure(figsize=(20, 20))
    axs = fig.subplots(4, 4)
    anchor_x_y = [0, 0.3, 0.6, 1]
    locations = ['right', 'left', 'top', 'bottom']
    shrink = 0.5

    for i, ax in enumerate(axs.flatten()):
        row = i // 4
        col = i % 4
        cs = ax.contourf(data)
        if row > 1:
            anchor = (anchor_x_y[col], 1)
        else:
            anchor = (1, anchor_x_y[col])
        cbar = plt.colorbar(
            cs, ax=ax, use_gridspec=use_gridspec,
            location=locations[row], anchor=anchor, shrink=shrink)
        ax.set_title(f'{locations[row]}, anchor={anchor}')
    return fig


# fig1 = anchored_cbar_position()
# fig1.savefig('demo1.png', dpi=300)
# fig2 = anchored_cbar_position(False)
# fig2.savefig('demo2.png', dpi=300)

This is the example I wrote to generate colorbar at different location with different anchor parameter.

Fig 1 gridspec = True
demo1

Fig2 gridspec = False
demo2

It can be confirmed that the position of colorbar when put at left and bottom, are identical between gridspec = True | False

when put at the right and top, the anchor position (anchor y value for right location, anchor x value for top) are also identicle, which means the p0 and anchor calculation in the PR is correct, and the same with the calculation in gridspec = False

However, we can also see the pad between colorbar axes and parent axes in the right and top scenario is different. I assume this is due to that make_axes uses different default pad parameter for this two locations. And make_axes_gridspec is using a fixed pad setting for all scenario, which has nothing to do with this PR.

Also, I am happy to try to fix this inconsistency. Should I try to do that in this PR? any suggestion?

@jklymak
Copy link
Member

jklymak commented Oct 10, 2020

I think this proves the PR is working. There are two todos, but I do wonder if they could just be follow up issues/PRs

  1. documentation for anchor/panchor could be improved. If @QuLogic is not following the meaning I don't think most people will.
  2. changing the gridspec upper and left pads to match non-gridspec. Obviously those are there to avoid decorators, and the real solution is to use constrained_layout.

I personally think these can be separate PRs but lets leave for a few hours for @QuLogic to weigh in.

@shawnchenx6
Copy link
Contributor Author

The pad difference between colorbar axes and parent axes in the right and top scenario is due to anchor parameter, not the pad.

See the following example:

def anchored_cbar_position(data, use_gridspec=True):
    # 4 * 4
    # each row using one location
    # each col using one anchor position
    fig = plt.figure(figsize=(20, 20))
    axs = fig.subplots(4, 4)
    anchor_x_y = [0, 0.3, 0.6, 1]
    locations = ['right', 'left', 'top', 'bottom']
    shrink = 0.5

    for i, ax in enumerate(axs.flatten()):
        row = i // 4
        col = i % 4
        cs = ax.contourf(data)
        if row == 0:
            anchor = (0, anchor_x_y[col])
        elif row == 1:
            anchor = (1, anchor_x_y[col])
        elif row == 2:
            anchor = (anchor_x_y[col], 0)
        elif row == 3:
            anchor = (anchor_x_y[col], 1)

        cbar = plt.colorbar(
            cs, ax=ax, use_gridspec=use_gridspec,
            location=locations[row], anchor=anchor, shrink=shrink)
        ax.set_title(f'{locations[row]}, anchor={anchor}')
    return fig


data = np.random.normal(loc=5, size=(30, 40))
fig1 = anchored_cbar_position(data)
fig1.savefig('demo1.png', dpi=300)
fig2 = anchored_cbar_position(data, False)
fig2.savefig('demo2.png', dpi=300)

The outputs are identical.
demo1

demo2

I think we should at least work on ‘top’ pad to avoid title being blocked.

@jklymak jklymak requested a review from QuLogic October 15, 2020 14:54
@QuLogic QuLogic dismissed their stale review October 16, 2020 04:13

Confirmed that this matches what the alternate mode does.

@QuLogic QuLogic merged commit 0ef97df into matplotlib:master Oct 16, 2020
@QuLogic
Copy link
Member

QuLogic commented Oct 16, 2020

Thanks @ShawnChen1996! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

I'll open a new issue for the remaining items, that I think can be done as a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.