-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Implementation of separate horizontal/vertical axes padding to the axis_grid toolkit #2466
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
Conversation
@@ -20,6 +20,13 @@ | |||
|
|||
#import numpy as np | |||
|
|||
def extend_axes_pad(value): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this probably should be _extend_axes_pad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, done.
Sorry if I am stepping on any toes by commenting on this code. This is similar in spirit to #2276 where I do something very similar (change a scalar parameter -> n-D) where I do it in the way I mention in the code comment (by adding extra scalar parameters). |
@leejjoon - pinging you on this one. I'm in favour of this change, but I haven't read it completely, and I suspect you should actually press the merge button once your happy with it. Cheers, |
self._horiz_pad_size.fixed_size = axes_pad | ||
self._vert_pad_size.fixed_size = axes_pad | ||
|
||
self._init_axes_pad(axes_pad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_init_axes_pad creates new Size.Fixed instances while set_axes_pad should not. Although I have not tested, I guess calling "set_axes_pad" will have no effect with this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this again, I no longer understand my comment as my claim that they are identical is clearly wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is fixed now
I have used this feature successfully when writing a scientific paper, see Fig. 6 |
@matejak This will need a re-base as it no longer merges cleanly. |
I have not followed the advice on the web when making the fork and the pull request (I was not aware of those at that time), so I will drop this request and create a new one. Sorry about that. |
@matejak What is the state of this? |
The state is good, there is another pull request #2731 |
This feature is useful in cases such as showing the same section of an image with different colorbar ranges.

Since colorbars have to be labelled, it is not possible to get things right when using the same horizontal and vertical spacing.
I attach an image (you can obtain it by running the demo script or by generating the documentation, the new functionality is the image to the right).
I think that I haven't broken anything since the documentation generation did not complain. However, I don't know how to write tests for this feature.