FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ENH: add ability to remove layout engine by tacaswell · Pull Request #22452 · matplotlib/matplotlib · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .py  (3) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
44 changes: 35 additions & 9 deletions lib/matplotlib/figure.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@

from matplotlib.axes import Axes, SubplotBase, subplot_class_factory
from matplotlib.gridspec import GridSpec
from matplotlib.layout_engine import (ConstrainedLayoutEngine,
TightLayoutEngine, LayoutEngine)
from matplotlib.layout_engine import (
ConstrainedLayoutEngine, TightLayoutEngine, LayoutEngine,
PlaceHolderLayoutEngine
)
import matplotlib.legend as mlegend
from matplotlib.patches import Rectangle
from matplotlib.text import Text
Expand Down Expand Up @@ -2382,7 +2384,9 @@ def _check_layout_engines_compat(self, old, new):
If the figure has used the old engine and added a colorbar then the
value of colorbar_gridspec must be the same on the new engine.
"""
if old is None or old.colorbar_gridspec == new.colorbar_gridspec:
if old is None or new is None:
return True
if old.colorbar_gridspec == new.colorbar_gridspec:
return True
# colorbar layout different, so check if any colorbars are on the
# figure...
Expand All @@ -2398,15 +2402,29 @@ def set_layout_engine(self, layout=None, **kwargs):

Parameters
----------
layout: {'constrained', 'compressed', 'tight'} or `~.LayoutEngine`
'constrained' will use `~.ConstrainedLayoutEngine`,
'compressed' will also use ConstrainedLayoutEngine, but with a
correction that attempts to make a good layout for fixed-aspect
ratio Axes. 'tight' uses `~.TightLayoutEngine`. Users and
libraries can define their own layout engines as well.
layout: {'constrained', 'compressed', 'tight', 'none'} or \
`LayoutEngine` or None

- 'constrained' will use `~.ConstrainedLayoutEngine`
- 'compressed' will also use `~.ConstrainedLayoutEngine`, but with
a correction that attempts to make a good layout for fixed-aspect
ratio Axes.
- 'tight' uses `~.TightLayoutEngine`
- 'none' removes layout engine.

If `None`, the behavior is controlled by :rc:`figure.autolayout`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Were we doing *None*?

tacaswell Aug 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We seem to have about the same number of both

$ ack  '\*None\*' -l | wc
     56      56    1865
$ ack  '`None`' -l | wc
     61      61    2223

[edit updated to exclude docs build product]

(which if `True` behaves as if 'tight' were passed) and
:rc:`figure.constrained_layout.use` (which if `True` behaves as if
'constrained' were passed). If both are `True`,
:rc:`figure.autolayout` takes priority.

Users and libraries can define their own layout engines and pass
the instance directly as well.

kwargs: dict
The keyword arguments are passed to the layout engine to set things
like padding and margin sizes. Only used if *layout* is a string.

"""
if layout is None:
if mpl.rcParams['figure.autolayout']:
Expand All @@ -2423,6 +2441,14 @@ def set_layout_engine(self, layout=None, **kwargs):
elif layout == 'compressed':
new_layout_engine = ConstrainedLayoutEngine(compress=True,
**kwargs)
elif layout == 'none':
if self._layout_engine is not None:
new_layout_engine = PlaceHolderLayoutEngine(
self._layout_engine.adjust_compatible,
self._layout_engine.colorbar_gridspec
)
else:
new_layout_engine = None
elif isinstance(layout, LayoutEngine):
new_layout_engine = layout
else:
Expand Down
24 changes: 24 additions & 0 deletions lib/matplotlib/layout_engine.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,30 @@ def execute(self, fig):
raise NotImplementedError


class PlaceHolderLayoutEngine(LayoutEngine):
"""
This layout engine does not adjust the figure layout at all.

The purpose of this `.LayoutEngine` is to act as a place holder when the
user removes a layout engine to ensure an incompatible `.LayoutEngine` can
not be set later.

Parameters
----------
adjust_compatible, colorbar_gridspec : bool
Allow the PlaceHolderLayoutEngine to mirror the behavior of whatever
layout engine it is replacing.

"""
def __init__(self, adjust_compatible, colorbar_gridspec, **kwargs):
self._adjust_compatible = adjust_compatible
self._colorbar_gridspec = colorbar_gridspec
super().__init__(**kwargs)

def execute(self, fig):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I guess I'm not sure about adding a Null engine here. It makes set_layout_engine('none') go through a (slightly) different codepath than never setting the layout engine at all. If we do this, we should probably initialize the figure with this NullLayoutEngine? But I'm not sure why we want to have this at all.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can not stick a formatter on at init time without making the bools tri-states ({unset, True, False}). This acts as a no-op place holder that remember the colorbar related settings without inventing another side-band way to store that.

Unfortunately I think that "never been set" vs "was set and then removed" are in fact different.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah I see - we don't want to switch colorbar behaviour after it has started to be used... So we are turning off the algorithm, but keeping its side effect.

At some point we need to figure out how to ditch the two ways of placing colorbars without breaking everybody.

return


class TightLayoutEngine(LayoutEngine):
"""
Implements the ``tight_layout`` geometry management. See
Expand Down
11 changes: 10 additions & 1 deletion lib/matplotlib/tests/test_figure.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from matplotlib.axes import Axes
from matplotlib.figure import Figure, FigureBase
from matplotlib.layout_engine import (ConstrainedLayoutEngine,
TightLayoutEngine)
TightLayoutEngine,
PlaceHolderLayoutEngine)
from matplotlib.ticker import AutoMinorLocator, FixedFormatter, ScalarFormatter
import matplotlib.pyplot as plt
import matplotlib.dates as mdates
Expand Down Expand Up @@ -578,6 +579,9 @@ def test_invalid_layouts():
fig, ax = plt.subplots(layout="constrained")
pc = ax.pcolormesh(np.random.randn(2, 2))
fig.colorbar(pc)
with pytest.raises(RuntimeError, match='Colorbar layout of new layout'):
fig.set_layout_engine("tight")
fig.set_layout_engine("none")
with pytest.raises(RuntimeError, match='Colorbar layout of new layout'):
fig.set_layout_engine("tight")

Expand All @@ -586,6 +590,11 @@ def test_invalid_layouts():
fig.colorbar(pc)
with pytest.raises(RuntimeError, match='Colorbar layout of new layout'):
fig.set_layout_engine("constrained")
fig.set_layout_engine("none")
assert isinstance(fig.get_layout_engine(), PlaceHolderLayoutEngine)

with pytest.raises(RuntimeError, match='Colorbar layout of new layout'):
fig.set_layout_engine("constrained")


@check_figures_equal(extensions=["png", "pdf"])
Expand Down

Back | FazBrowse Home | New Git URL