Uses tight_layout.get_subplotspec_list to check if all axes are compatible w/ tight_layout #1170

Merged
merged 2 commits into from Sep 1, 2012

Conversation

Projects
None yet
4 participants
@leejjoon
Contributor

leejjoon commented Aug 29, 2012

An axes that uses axes_locator (even though the axes itself is not an instance of SubplotBase) can be compatible with tight_layout. This PR suppresses warning message for such case.

lib/matplotlib/figure.py
- subplot_axes = [ax for ax in self.axes if isinstance(ax, SubplotBase)]
- if len(subplot_axes) < len(self.axes):
+ if None in get_subplotspec_list(self.axes):
warnings.warn("tight_layout can only process Axes that descend "
"from SubplotBase; results might be incorrect.")

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Aug 29, 2012

Contributor

@mdboom wanted to replace "descend" with "inherit" in the warning text.. you could change that in this PR so mdboom doesn't have to do it

@pwuertz

pwuertz Aug 29, 2012

Contributor

@mdboom wanted to replace "descend" with "inherit" in the warning text.. you could change that in this PR so mdboom doesn't have to do it

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Aug 30, 2012

Owner

But with this change, the warning itself is now incorrect and must be fixed. @leejjoon, should it say something like "This figure includes Axes that do not inherit from SubplotBase and/or were not created using axes_locator; tight_layout will not take such Axes into account, so its results might be incorrect."

@efiring

efiring Aug 30, 2012

Owner

But with this change, the warning itself is now incorrect and must be fixed. @leejjoon, should it say something like "This figure includes Axes that do not inherit from SubplotBase and/or were not created using axes_locator; tight_layout will not take such Axes into account, so its results might be incorrect."

lib/matplotlib/figure.py
@@ -1422,17 +1422,17 @@ def tight_layout(self, renderer=None, pad=1.08, h_pad=None, w_pad=None, rect=Non
labels) will fit into. Default is (0, 0, 1, 1).
"""
- from tight_layout import get_renderer, get_tight_layout_figure
+ from tight_layout import get_renderer, get_tight_layout_figure, \

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Aug 30, 2012

Owner

Style police: No backslash line continuations, please. You may use parentheses here instead.

@efiring

efiring Aug 30, 2012

Owner

Style police: No backslash line continuations, please. You may use parentheses here instead.

lib/matplotlib/figure.py
warnings.warn("tight_layout can only process Axes that descend "
"from SubplotBase; results might be incorrect.")
if renderer is None:
renderer = get_renderer(self)
- kwargs = get_tight_layout_figure(self, subplot_axes, renderer,
+ kwargs = get_tight_layout_figure(self, self.axes, renderer,

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Aug 30, 2012

Owner

Now we are having to call get_subplotspec_list twice, once to figure out whether to issue a warning, and a second time inside of get_tight_layout_figure. This is not good at all. Either save and reuse the calculation here, or maybe move the warning into get_tight_layout_figure.

@efiring

efiring Aug 30, 2012

Owner

Now we are having to call get_subplotspec_list twice, once to figure out whether to issue a warning, and a second time inside of get_tight_layout_figure. This is not good at all. Either save and reuse the calculation here, or maybe move the warning into get_tight_layout_figure.

lib/matplotlib/tight_layout.py
+ else:
+ subplotspec = None
+
+ if subplotspec is not None and \

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Aug 30, 2012

Owner

PEP8: please use parentheses, not backslash, for line continuation.

@efiring

efiring Aug 30, 2012

Owner

PEP8: please use parentheses, not backslash, for line continuation.

lib/matplotlib/tight_layout.py
@@ -209,6 +209,33 @@ def get_renderer(fig):
return renderer
+def get_subplotspec_list(axes_list):

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Aug 30, 2012

Owner

I worry about the complexity here. Apart from the difficulty in understanding what is going on, there is the fact that tight_layout is called on every draw().

@efiring

efiring Aug 30, 2012

Owner

I worry about the complexity here. Apart from the difficulty in understanding what is going on, there is the fact that tight_layout is called on every draw().

@efiring

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Aug 30, 2012

Owner

Can all this be simplified by having an Axes attribute to indicate whether tight_layout is supported? The attribute would be set to True whenever an operation creates a suitable Axes, or perhaps (if this is possible) modifies an existing Axes in such a way as to support tight_layout?

Owner

efiring commented Aug 30, 2012

Can all this be simplified by having an Axes attribute to indicate whether tight_layout is supported? The attribute would be set to True whenever an operation creates a suitable Axes, or perhaps (if this is possible) modifies an existing Axes in such a way as to support tight_layout?

@leejjoon

This comment has been minimized.

Show comment Hide comment
@leejjoon

leejjoon Aug 30, 2012

Contributor

While I agree that we need to simplify this, I don't have good solution for now. I will think about it, and any suggestion will be welcomed.

Contributor

leejjoon commented Aug 30, 2012

While I agree that we need to simplify this, I don't have good solution for now. I will think about it, and any suggestion will be welcomed.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 30, 2012

This pull request fails (merged 1d80e7f into 94c53e1).

This pull request fails (merged 1d80e7f into 94c53e1).

efiring added a commit that referenced this pull request Sep 1, 2012

Merge pull request #1170 from leejjoon/fix-tight-layout-condition
Uses tight_layout.get_subplotspec_list to check if all axes are compatible w/ tight_layout

@efiring efiring merged commit 2c91aa6 into matplotlib:master Sep 1, 2012

@efiring

This comment has been minimized.

Show comment Hide comment
@efiring

efiring Sep 1, 2012

Owner

This is at least an interim solution, so I merged it.

Owner

efiring commented Sep 1, 2012

This is at least an interim solution, so I merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment