Skip to content

update colorbar.py make_axes_gridspec#16646

Closed
shawnchenx6 wants to merge 5 commits into
matplotlib:masterfrom
shawnchenx6:master
Closed

update colorbar.py make_axes_gridspec#16646
shawnchenx6 wants to merge 5 commits into
matplotlib:masterfrom
shawnchenx6:master

Conversation

@shawnchenx6

@shawnchenx6 shawnchenx6 commented Mar 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@jklymak jklymak self-assigned this Mar 3, 2020
@jklymak

jklymak commented Mar 3, 2020

Copy link
Copy Markdown
Member

Thanks a lot for the PR. After a quick look this seems feasible and reasonable. Will probably need some tests, and you have a flake8 error (follow links under Travis CI). Can you also check that this works w/ constrained_layout?

@jklymak jklymak added this to the v3.3.0 milestone Mar 3, 2020
Update colorbar.py
0991429 
should fix the pep8 failure
@shawnchenx6

shawnchenx6 commented Mar 3, 2020

Copy link
Copy Markdown
Contributor Author

I have made a another commit, should fix the format error.
I may need some help on checking the constrained_layout

@jklymak

jklymak commented Apr 4, 2020

Copy link
Copy Markdown
Member

Sorry, I forgot about this. This is not compatible with constrained_layout, but that need not prevent it, and indeed maybe constrained_layout should be fixed to do this.

This needs tests and a what's new entry, but I don't see any reason why adding this would be a problem.

@jklymak

jklymak commented Apr 4, 2020

Copy link
Copy Markdown
Member

BTW< don't be shy about pinging for review at a reasonable interval.

@shawnchenx6

shawnchenx6 commented Apr 7, 2020

Copy link
Copy Markdown
Contributor Author

Thank you for pushing this PR forward, this is my 1st time to contribute to matplotlib, thus not very familiar with the procedure. What do I do next? I can see it fails the codecov test.

@jklymak

jklymak commented Apr 7, 2020

Copy link
Copy Markdown
Member

If you can, it would be nice if you would add some tests, probably in lib/matplotlib/tests/test_colorbar.py. Preferably without using image comparison, which we are trying to avoid. In this case, you can probably just check that the anchored position of the colorbar is correct without resorting to an image test.

@QuLogic QuLogic modified the milestones: v3.3.0, v3.4.0 May 5, 2020
@shawnchenx6

shawnchenx6 commented May 23, 2020

Copy link
Copy Markdown
Contributor Author

Sorry for the delay.
I have added test for colorbar postioning.
Please tell me if there is anything I need to do.
@jklymak

@jklymak

jklymak commented Jul 17, 2020

Copy link
Copy Markdown
Member

Sorry, this needs a rebase, and the docs should build as well. Please feel free to ping - I think this is a good addition.

@jklymak

jklymak commented Jul 17, 2020

Copy link
Copy Markdown
Member
  • oh, and this needs at least an update to the docstring for colorbar., but an example would be nice as well...

@shawnchenx6

shawnchenx6 commented Aug 24, 2020

Copy link
Copy Markdown
Contributor Author

I rebased my commits into a branch named colorbarGridPostion, however, I don't know how to change this PR to merge from that branch, is there anyone who can help me?

I will work on the docstring and example very soon.

@jklymak

jklymak commented Aug 24, 2020

Copy link
Copy Markdown
Member

Since it was just me who has reviewed this so far, why not make things easier on ourselves and just open a new PR... Its better that you have the commits in a branch other than master anyhow. Thanks...

@shawnchenx6

shawnchenx6 commented Aug 24, 2020

Copy link
Copy Markdown
Contributor Author

new PR done.

Has merge conflicts

@jklymak

jklymak commented Aug 24, 2020

Copy link
Copy Markdown
Member

Closed in lieu of #18340

@jklymak jklymak closed this Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jklymak jklymak

Labels

status: duplicate status: needs documentation status: needs rebase topic: color/colorbar

Projects

None yet

Milestone

v3.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@shawnchenx6 @jklymak @QuLogic