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

update colorbar.py make_axes_gridspec #16646

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

Closed
wants to merge 5 commits into from

Conversation

shawnchenx6
Copy link
Contributor

@shawnchenx6 shawnchenx6 commented Mar 3, 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

@jklymak jklymak self-assigned this Mar 3, 2020
@jklymak
Copy link
Member

jklymak commented Mar 3, 2020

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
should fix the pep8 failure
@shawnchenx6
Copy link
Contributor Author

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

@jklymak
Copy link
Member

jklymak commented Apr 4, 2020

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
Copy link
Member

jklymak commented Apr 4, 2020

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

@shawnchenx6
Copy link
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
Copy link
Member

jklymak commented Apr 7, 2020

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
Copy link
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
Copy link
Member

jklymak commented Jul 17, 2020

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
Copy link
Member

jklymak commented Jul 17, 2020

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

@shawnchenx6
Copy link
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
Copy link
Member

jklymak commented Aug 24, 2020

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
Copy link
Contributor Author

new PR done.

Has merge conflicts

@jklymak
Copy link
Member

jklymak commented Aug 24, 2020

Closed in lieu of #18340

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.

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