Skip to content
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

Implementation of separate horizontal/vertical axes padding to the axis_grid toolkit #2466

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 38 additions & 5 deletions examples/axes_grid/demo_axes_grid.py
Expand Up @@ -14,7 +14,7 @@ def demo_simple_grid(fig):
A grid of 2x2 images with 0.05 inch pad between images and only
the lower-left axes is labeled.
"""
grid = AxesGrid(fig, 131, # similar to subplot(131)
grid = AxesGrid(fig, 141, # similar to subplot(141)
nrows_ncols = (2, 2),
axes_pad = 0.05,
label_mode = "1",
Expand All @@ -33,7 +33,7 @@ def demo_grid_with_single_cbar(fig):
"""
A grid of 2x2 images with a single colorbar
"""
grid = AxesGrid(fig, 132, # similar to subplot(132)
grid = AxesGrid(fig, 142, # similar to subplot(142)
nrows_ncols = (2, 2),
axes_pad = 0.0,
share_all=True,
Expand Down Expand Up @@ -61,7 +61,7 @@ def demo_grid_with_each_cbar(fig):
A grid of 2x2 images. Each image has its own colorbar.
"""

grid = AxesGrid(F, 133, # similar to subplot(122)
grid = AxesGrid(F, 143, # similar to subplot(143)
nrows_ncols = (2, 2),
axes_pad = 0.1,
label_mode = "1",
Expand All @@ -83,16 +83,49 @@ def demo_grid_with_each_cbar(fig):
grid.axes_llc.set_xticks([-2, 0, 2])
grid.axes_llc.set_yticks([-2, 0, 2])

def demo_grid_with_each_cbar_labelled(fig):
"""
A grid of 2x2 images. Each image has its own colorbar.
"""

grid = AxesGrid(F, 144, # similar to subplot(144)
nrows_ncols = (2, 2),
axes_pad = ( 0.45, 0.15),
label_mode = "1",
share_all = True,
cbar_location="right",
cbar_mode="each",
cbar_size="7%",
cbar_pad="2%",
)
Z, extent = get_demo_image()

# Use a different colorbar range every time
limits = ((0, 1), (-2, 2), (-1.7, 1.4), (-1.5, 1))
for i in range(4):
im = grid[i].imshow(Z, extent=extent, interpolation="nearest",
vmin = limits[i][0], vmax = limits[i][1])
grid.cbar_axes[i].colorbar(im)

for i, cax in enumerate(grid.cbar_axes):
cax.set_yticks((limits[i][0], limits[i][1]))

# This affects all axes because we set share_all = True.
grid.axes_llc.set_xticks([-2, 0, 2])
grid.axes_llc.set_yticks([-2, 0, 2])




if 1:
F = plt.figure(1, (5.5, 2.5))
F = plt.figure(1, (10.5, 2.5))

F.subplots_adjust(left=0.05, right=0.98)
F.subplots_adjust(left=0.05, right=0.95)

demo_simple_grid(F)
demo_grid_with_single_cbar(F)
demo_grid_with_each_cbar(F)
demo_grid_with_each_cbar_labelled(F)

plt.draw()
plt.show()
Expand Down
37 changes: 30 additions & 7 deletions lib/mpl_toolkits/axes_grid1/axes_grid.py
Expand Up @@ -20,6 +20,13 @@

#import numpy as np

def extend_axes_pad(value):
Copy link
Member

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

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 agree, done.

# Check whether a list/tuple/array or scalar has been passed
ret = value
if not hasattr(ret, "__getitem__"):
ret = (value, value)
return ret

def _tick_only(ax, bottom_on, left_on):
bottom_off = not bottom_on
left_off = not left_on
Expand Down Expand Up @@ -201,6 +208,8 @@ def __init__(self, fig,
================ ======== =========================================
direction "row" [ "row" | "column" ]
axes_pad 0.02 float| pad between axes given in inches
or tuple-like of floats,
(horizontal padding, vertical padding)
add_all True [ True | False ]
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to add an axes_pad_vert (or similarly named) which defaults to None -> set equal to axes_pad else, axes_pad is used for horizontal and axes_pad_vert is used for vertical? It eliminates the need for the extend function and leads to simpler api and code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although I don't insist on anything, there are some good reasons for my approach:

  • It is similar to the numpy pattern, where you can write either np.zeros(4) or np.zeros((4,)) for a 1D array, but you have to write np.zeros((4,2)) for a 2D array. Allowing passing a scalar instead of a tuple makes it more convenient for common cases, although it complicates implementation
  • This approach won't break backward compatibility (adding the axes_pad_vert argument in the middle would, adding it at the end wouldn't, but that would lack some sense)

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. I had been thinking about them as pure kwargs and forgot about calling with only positional arguments, I now agree as written is the right way to do this.

share_all False [ True | False ]
share_x True [ True | False ]
Expand Down Expand Up @@ -314,10 +323,11 @@ def __init__(self, fig,


def _init_axes_pad(self, axes_pad):
axes_pad = extend_axes_pad(axes_pad)
self._axes_pad = axes_pad

self._horiz_pad_size = Size.Fixed(axes_pad)
self._vert_pad_size = Size.Fixed(axes_pad)
self._horiz_pad_size = Size.Fixed(axes_pad[0])
self._vert_pad_size = Size.Fixed(axes_pad[1])


def _update_locators(self):
Expand Down Expand Up @@ -382,14 +392,22 @@ def get_geometry(self):

def set_axes_pad(self, axes_pad):
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have both of these functions? They are identical, one should either be an alias for the other.

Copy link
Member

Choose a reason for hiding this comment

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

The other function I am referring to is _init_axes_pad which is obvious on the diff page, but very not obvious on the discussion page.

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 don't know why we have them, so I have done as you suggested them.

"set axes_pad"
axes_pad = extend_axes_pad(axes_pad)
self._axes_pad = axes_pad

self._horiz_pad_size.fixed_size = axes_pad
self._vert_pad_size.fixed_size = axes_pad
self._horiz_pad_size.fixed_size = axes_pad[0]
self._vert_pad_size.fixed_size = axes_pad[1]


def get_axes_pad(self):
"get axes_pad"
"""
get axes_pad

Returns
-------
tuple
Padding in inches, (horizontal pad, vertical pad)
"""
return self._axes_pad

def set_aspect(self, aspect):
Expand Down Expand Up @@ -495,6 +513,8 @@ def __init__(self, fig,
================ ======== =========================================
direction "row" [ "row" | "column" ]
axes_pad 0.02 float| pad between axes given in inches
or tuple-like of floats
(horizontal padding, vertical padding)
add_all True [ True | False ]
share_all False [ True | False ]
aspect True [ True | False ]
Expand All @@ -521,12 +541,15 @@ def __init__(self, fig,

self.ngrids = ngrids

axes_pad = extend_axes_pad(axes_pad)
self._axes_pad = axes_pad

self._colorbar_mode = cbar_mode
self._colorbar_location = cbar_location
if cbar_pad is None:
self._colorbar_pad = axes_pad
# horizontal or vertical arrangement?
self._colorbar_pad = axes_pad[0] \
if cbar_location in ("left", "right") else axes_pad[1]
else:
self._colorbar_pad = cbar_pad

Expand Down Expand Up @@ -688,7 +711,7 @@ def _update_locators(self):
v_ax_pos = []
v_cb_pos = []
for row,ax in enumerate(self._row_refax[::-1]):
if v: v.append(self._horiz_pad_size) #Size.Fixed(self._axes_pad))
if v: v.append(self._vert_pad_size) #Size.Fixed(self._axes_pad))
Copy link
Member

Choose a reason for hiding this comment

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

can you fix the one-line if statements (and general pep8) while you are working on this file?

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 have addressed it.
By the way, there is quite a huge code duplication between Grid and ImageGrid __init__ methods, but it can't be resolved easily. Adapting Grid to accept aspect argument would help, but it is commented out, so there may be a reason why this is not possible.


if ax:
sz = Size.AxesY(ax)
Expand Down