Fix for issue #955 #1003

Closed
wants to merge 1 commit into
from

Projects

None yet

5 participants

@oxling
Contributor
oxling commented Jul 10, 2012

The Y axis will reset its aspect ratio to the aspect of its parent axes when get_size is called.

@oxling oxling Fix for issue #955
The Y axis will reset its aspect ratio to the aspect of its parent axes when get_size is called.
141716e
Member

Personally, I would prefer that the _aspect property get set properly in the first place upon changes to the axes. Maybe the axes object should update the _aspect property of any images it may have whenever its aspect changes?

Contributor
oxling commented Jul 10, 2012

That's true, it would be cleaner. Are the child Axes aware of their parent ImageGrid? From what I can tell you need to fix the aspect of the AxesY object, which is a member of ImageGrid and not visible to its Axes.

Does mpl_toolkits.axes_grid1.axes_size.AxesY._aspect have a unique meaning other than the axes aspect? If not it would be cleaner to not have a private duplicate copy of this parameter at all, and just pull form the axes object itself - like what is done for the data limits.

Member

@oxling: You have your conceptual hierarchy backwards. At the top is a Figure object, which has Axes objects as children, and an Axes object has AxesImage objects as children. All artist objects are aware of what Axes they are attached to, and all Axes objects keep lists of the various kinds of artists attached to it for book-keeping purposes.

An Axes object has members self.images as a list of AxesImage objects (although it is maintained and populated differently from the bookkeeping lists such as self.artists, self.collections, etc. for reasons that I am not aware of).

With respect to ImageGrid and anything else in mpl_toolkits, one must understand that the modules in mpl_toolkits are considered to be just outside the core matplotlib package (even though we ship it with it, and test it as a part of the whole). ImageGrid depends on mpl core, not the reverse. The problem we are experiencing here is a problem in the core of mpl. Hopefully, fixing it there should then only require minimal changes in AxesGrid to bring it up to speed with mpl core.

Member

@aeyea, I think @leejjoon would be in the best position to answer that question.

I think oxling is referring to the ImageGrid->SubplotDivider->LocatableAxes hierarchy (e.g. grid._divider._vertical[0]._axes).

The AxesImage core objects don't have any _aspect property. It's the AxesDivider code in mpl_toolkits that's calculating internally the wrong geometry for the aspect != 1 Axes it manages, which it does without polling any of the axes children.

The bug can be reproduced without creating any images - just changing the aspect of one of the axes:

import matplotlib.pyplot as plt
from mpl_toolkits.axes_grid1 import ImageGrid
import numpy as np

im = np.arange(100)
im.shape = 10, 10

fig = plt.figure(1, (4., 4.))
grid = ImageGrid(fig, 111, # similar to subplot(111)
                nrows_ncols = (2, 2), # creates 2x2 grid of axes
                axes_pad=0.1, # pad between axes in inch.
                )

grid[0].set_aspect(2.0)
plt.show(); plt.show(); # ...
Contributor
oxling commented Jul 13, 2012

You're right @aeyea, that's the object I was referring to.

@leejjoon leejjoon was assigned Jul 17, 2012
Member
pelson commented Jul 28, 2012

I've not read through any of this code, but I would like to make the statement that it is very strange that a getter (get_size) should have a side effect in the way that it appears it does.

@pelson pelson commented on the diff Aug 19, 2012
lib/mpl_toolkits/axes_grid1/axes_size.py
@@ -99,6 +99,9 @@ def __init__(self, axes, aspect=1.):
self._aspect = aspect
def get_size(self, renderer):
+ #Reset the aspect. The grid aspect may have been changed.
+ self._aspect = self._axes.get_aspect()
pelson
pelson Aug 19, 2012 Member

Couldn't the result of get_aspect be the string 'auto'?

Member
pelson commented Nov 29, 2012

@leejjoon : I think you are the best person to help with this PR, would you be able to comment on this change?

Contributor

I don't think this PR fully address the underlying issue, and I believe I have fixed the problem with f6b344e. There has been a related discussion with #955. Given that there has been no side-effects reported so far, I will go ahead and close both PRs. If there still is a problem, please reopen the issue.

@leejjoon leejjoon closed this Nov 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment