From b7ccfb66066e538e57f0792dee1525b041e92d5c Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Fri, 4 Sep 2020 15:43:28 -0400 Subject: [PATCH 1/3] FIX/API: `fig.canvas.draw` always updates internal state Previously the non-interactive backends, other than Agg, did not define `draw` methods and fell back to the base no-op version. However, we have been documenting that the correct way to update the various internal state we keep (run tight/constrained layouts, auto limits, text size/position, ...), this is now a bug due to our suggested usage drifting. closes #18407 --- lib/matplotlib/backend_bases.py | 13 +++++++-- lib/matplotlib/backends/backend_pdf.py | 5 +++- lib/matplotlib/backends/backend_pgf.py | 7 ++++- lib/matplotlib/backends/backend_ps.py | 5 +++- lib/matplotlib/backends/backend_svg.py | 5 +++- lib/matplotlib/tests/test_backend_bases.py | 33 ++++++++++++++++++++++ 6 files changed, 62 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py index a2656c794f69..d6269e24a703 100644 --- a/lib/matplotlib/backend_bases.py +++ b/lib/matplotlib/backend_bases.py @@ -1584,6 +1584,12 @@ def _draw(renderer): raise Done(renderer) figure.canvas = orig_canvas +def _no_output_draw(figure): + renderer = _get_renderer(figure) + with renderer._draw_disabled(): + figure.draw(renderer) + + def _is_non_interactive_terminal_ipython(ip): """ Return whether we are in a a terminal IPython, but non interactive. @@ -1621,7 +1627,9 @@ def _check_savefig_extra_args(func=None, extra_kwargs=()): @functools.wraps(func) def wrapper(*args, **kwargs): name = 'savefig' # Reasonable default guess. - public_api = re.compile(r'^savefig|print_[A-Za-z0-9]+$') + public_api = re.compile( + r'^savefig|print_[A-Za-z0-9]+|_no_output_draw$' + ) seen_print_figure = False for frame, line in traceback.walk_stack(None): if frame is None: @@ -1632,8 +1640,9 @@ def wrapper(*args, **kwargs): frame.f_globals.get('__name__', '')): if public_api.match(frame.f_code.co_name): name = frame.f_code.co_name - if name == 'print_figure': + if name in ('print_figure', '_no_output_draw'): seen_print_figure = True + else: break diff --git a/lib/matplotlib/backends/backend_pdf.py b/lib/matplotlib/backends/backend_pdf.py index fb6c59c69244..2f5fb2bd990f 100644 --- a/lib/matplotlib/backends/backend_pdf.py +++ b/lib/matplotlib/backends/backend_pdf.py @@ -28,7 +28,7 @@ from matplotlib._pylab_helpers import Gcf from matplotlib.backend_bases import ( _Backend, _check_savefig_extra_args, FigureCanvasBase, FigureManagerBase, - GraphicsContextBase, RendererBase) + GraphicsContextBase, RendererBase, _no_output_draw) from matplotlib.backends.backend_mixed import MixedModeRenderer from matplotlib.figure import Figure from matplotlib.font_manager import findfont, get_font @@ -2730,6 +2730,9 @@ def print_pdf(self, filename, *, else: # we opened the file above; now finish it off file.close() + def draw(self): + _no_output_draw(self.figure) + return super().draw() FigureManagerPdf = FigureManagerBase diff --git a/lib/matplotlib/backends/backend_pgf.py b/lib/matplotlib/backends/backend_pgf.py index e59290338ac5..9d01f603e021 100644 --- a/lib/matplotlib/backends/backend_pgf.py +++ b/lib/matplotlib/backends/backend_pgf.py @@ -19,7 +19,8 @@ from matplotlib import _api, cbook, font_manager as fm from matplotlib.backend_bases import ( _Backend, _check_savefig_extra_args, FigureCanvasBase, FigureManagerBase, - GraphicsContextBase, RendererBase) + GraphicsContextBase, RendererBase, _no_output_draw +) from matplotlib.backends.backend_mixed import MixedModeRenderer from matplotlib.backends.backend_pdf import ( _create_pdf_info_dict, _datetime_to_pdf) @@ -906,6 +907,10 @@ def print_png(self, fname_or_fh, *args, **kwargs): def get_renderer(self): return RendererPgf(self.figure, None) + def draw(self): + _no_output_draw(self.figure) + return super().draw() + FigureManagerPgf = FigureManagerBase diff --git a/lib/matplotlib/backends/backend_ps.py b/lib/matplotlib/backends/backend_ps.py index bfd8324fbd26..cca4190a6778 100644 --- a/lib/matplotlib/backends/backend_ps.py +++ b/lib/matplotlib/backends/backend_ps.py @@ -23,7 +23,7 @@ from matplotlib.afm import AFM from matplotlib.backend_bases import ( _Backend, _check_savefig_extra_args, FigureCanvasBase, FigureManagerBase, - GraphicsContextBase, RendererBase) + GraphicsContextBase, RendererBase, _no_output_draw) from matplotlib.cbook import is_writable_file_like, file_requires_unicode from matplotlib.font_manager import get_font from matplotlib.ft2font import LOAD_NO_HINTING, LOAD_NO_SCALE @@ -1129,6 +1129,9 @@ def _print_figure_tex( _move_path_to_path_or_stream(tmpfile, outfile) + def draw(self): + _no_output_draw(self.figure) + return super().draw() def convert_psfrags(tmpfile, psfrags, font_preamble, custom_preamble, paper_width, paper_height, orientation): diff --git a/lib/matplotlib/backends/backend_svg.py b/lib/matplotlib/backends/backend_svg.py index ac925d0b0777..401d7d91268a 100644 --- a/lib/matplotlib/backends/backend_svg.py +++ b/lib/matplotlib/backends/backend_svg.py @@ -17,7 +17,7 @@ from matplotlib import _api, cbook from matplotlib.backend_bases import ( _Backend, _check_savefig_extra_args, FigureCanvasBase, FigureManagerBase, - RendererBase) + RendererBase, _no_output_draw) from matplotlib.backends.backend_mixed import MixedModeRenderer from matplotlib.colors import rgb2hex from matplotlib.dates import UTC @@ -1363,6 +1363,9 @@ def _print_svg(self, filename, fh, *, dpi=None, bbox_inches_restore=None, def get_default_filetype(self): return 'svg' + def draw(self): + _no_output_draw(self.figure) + return super().draw() FigureManagerSVG = FigureManagerBase diff --git a/lib/matplotlib/tests/test_backend_bases.py b/lib/matplotlib/tests/test_backend_bases.py index 77f6bebd262b..eb573d0aed0d 100644 --- a/lib/matplotlib/tests/test_backend_bases.py +++ b/lib/matplotlib/tests/test_backend_bases.py @@ -187,3 +187,36 @@ def test_toolbar_zoompan(): assert ax.get_navigate_mode() == "ZOOM" ax.figure.canvas.manager.toolmanager.trigger_tool('pan') assert ax.get_navigate_mode() == "PAN" + + +@pytest.mark.parametrize("backend", ['svg', 'pgf', 'ps', 'pdf']) +def test_draw(backend): + from matplotlib.figure import Figure + from matplotlib.backends.backend_agg import FigureCanvas + test_backend = pytest.importorskip( + f'matplotlib.backends.backend_{backend}' + ) + TestCanvas = test_backend.FigureCanvas + fig_test = Figure(constrained_layout=True) + TestCanvas(fig_test) + axes_test = fig_test.subplots(2, 2) + + # defaults to FigureCanvasBase + fig_agg = Figure(constrained_layout=True) + # put a backends.backend_agg.FigureCanvas on it + FigureCanvas(fig_agg) + axes_agg = fig_agg.subplots(2, 2) + + init_pos = [ax.get_position() for ax in axes_test.ravel()] + + fig_test.canvas.draw() + fig_agg.canvas.draw() + + layed_out_pos_test = [ax.get_position() for ax in axes_test.ravel()] + layed_out_pos_agg = [ax.get_position() for ax in axes_agg.ravel()] + + for init, placed in zip(init_pos, layed_out_pos_test): + assert not np.allclose(init, placed, atol=0.005) + + for ref, test in zip(layed_out_pos_agg, layed_out_pos_test): + np.testing.assert_allclose(ref, test, atol=0.005) From 1173770f3319c053b334b8f86f574aeb7a77fba1 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Mon, 7 Sep 2020 13:55:12 -0400 Subject: [PATCH 2/3] MNT: hoist latex helpers from pgf tests to matplotlib.testing Move the marks to skip if various latex installs are missing to be visible on other modules. --- lib/matplotlib/backends/backend_ps.py | 1 + lib/matplotlib/testing/__init__.py | 31 +++++++++++++++++++++ lib/matplotlib/tests/test_backend_bases.py | 8 +++++- lib/matplotlib/tests/test_backend_pgf.py | 32 ++++------------------ 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/lib/matplotlib/backends/backend_ps.py b/lib/matplotlib/backends/backend_ps.py index cca4190a6778..5dbc841adc92 100644 --- a/lib/matplotlib/backends/backend_ps.py +++ b/lib/matplotlib/backends/backend_ps.py @@ -1133,6 +1133,7 @@ def draw(self): _no_output_draw(self.figure) return super().draw() + def convert_psfrags(tmpfile, psfrags, font_preamble, custom_preamble, paper_width, paper_height, orientation): """ diff --git a/lib/matplotlib/testing/__init__.py b/lib/matplotlib/testing/__init__.py index 69a6985c8e96..274adbc671b7 100644 --- a/lib/matplotlib/testing/__init__.py +++ b/lib/matplotlib/testing/__init__.py @@ -4,6 +4,9 @@ import locale import logging +import subprocess +from pathlib import Path +from tempfile import TemporaryDirectory import matplotlib as mpl from matplotlib import _api @@ -44,3 +47,31 @@ def setup(): # are not necessarily the default values as specified in rcsetup.py. set_font_settings_for_testing() set_reproducibility_for_testing() + + +def check_for_pgf(texsystem): + """ + Check if a given TeX system + pgf is available + + Parameters + ---------- + texsystem : str + The executable name to check + """ + with TemporaryDirectory() as tmpdir: + tex_path = Path(tmpdir, "test.tex") + tex_path.write_text(r""" + \documentclass{minimal} + \usepackage{pgf} + \begin{document} + \typeout{pgfversion=\pgfversion} + \makeatletter + \@@end + """) + try: + subprocess.check_call( + [texsystem, "-halt-on-error", str(tex_path)], cwd=tmpdir, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + except (OSError, subprocess.CalledProcessError): + return False + return True diff --git a/lib/matplotlib/tests/test_backend_bases.py b/lib/matplotlib/tests/test_backend_bases.py index eb573d0aed0d..c887fc8e8de5 100644 --- a/lib/matplotlib/tests/test_backend_bases.py +++ b/lib/matplotlib/tests/test_backend_bases.py @@ -1,5 +1,6 @@ import re +from matplotlib.testing import check_for_pgf from matplotlib.backend_bases import ( FigureCanvasBase, LocationEvent, MouseButton, MouseEvent, NavigationToolbar2, RendererBase) @@ -13,6 +14,9 @@ import numpy as np import pytest +needs_xelatex = pytest.mark.skipif(not check_for_pgf('xelatex'), + reason='xelatex + pgf is required') + def test_uses_per_path(): id = transforms.Affine2D() @@ -189,7 +193,9 @@ def test_toolbar_zoompan(): assert ax.get_navigate_mode() == "PAN" -@pytest.mark.parametrize("backend", ['svg', 'pgf', 'ps', 'pdf']) +@pytest.mark.parametrize( + "backend", ['svg', 'ps', 'pdf', pytest.param('pgf', marks=needs_xelatex)] +) def test_draw(backend): from matplotlib.figure import Figure from matplotlib.backends.backend_agg import FigureCanvas diff --git a/lib/matplotlib/tests/test_backend_pgf.py b/lib/matplotlib/tests/test_backend_pgf.py index e3d29a9c434c..8b62f0529689 100644 --- a/lib/matplotlib/tests/test_backend_pgf.py +++ b/lib/matplotlib/tests/test_backend_pgf.py @@ -1,16 +1,15 @@ import datetime from io import BytesIO import os -from pathlib import Path import shutil import subprocess -from tempfile import TemporaryDirectory import numpy as np import pytest import matplotlib as mpl import matplotlib.pyplot as plt +from matplotlib.testing import check_for_pgf from matplotlib.testing.compare import compare_images, ImageComparisonFailure from matplotlib.backends.backend_pgf import PdfPages, common_texification from matplotlib.testing.decorators import (_image_directories, @@ -19,32 +18,11 @@ baseline_dir, result_dir = _image_directories(lambda: 'dummy func') - -def check_for(texsystem): - with TemporaryDirectory() as tmpdir: - tex_path = Path(tmpdir, "test.tex") - tex_path.write_text(r""" - \documentclass{minimal} - \usepackage{pgf} - \begin{document} - \typeout{pgfversion=\pgfversion} - \makeatletter - \@@end - """) - try: - subprocess.check_call( - [texsystem, "-halt-on-error", str(tex_path)], cwd=tmpdir, - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - except (OSError, subprocess.CalledProcessError): - return False - return True - - -needs_xelatex = pytest.mark.skipif(not check_for('xelatex'), +needs_xelatex = pytest.mark.skipif(not check_for_pgf('xelatex'), reason='xelatex + pgf is required') -needs_pdflatex = pytest.mark.skipif(not check_for('pdflatex'), +needs_pdflatex = pytest.mark.skipif(not check_for_pgf('pdflatex'), reason='pdflatex + pgf is required') -needs_lualatex = pytest.mark.skipif(not check_for('lualatex'), +needs_lualatex = pytest.mark.skipif(not check_for_pgf('lualatex'), reason='lualatex + pgf is required') needs_ghostscript = pytest.mark.skipif( "eps" not in mpl.testing.compare.converter, @@ -341,7 +319,7 @@ def test_unknown_font(caplog): @pytest.mark.parametrize("texsystem", ("pdflatex", "xelatex", "lualatex")) @pytest.mark.backend("pgf") def test_minus_signs_with_tex(fig_test, fig_ref, texsystem): - if not check_for(texsystem): + if not check_for_pgf(texsystem): pytest.skip(texsystem + ' + pgf is required') mpl.rcParams["pgf.texsystem"] = texsystem fig_test.text(.5, .5, "$-1$") From 22e84f6c6a3338f56e4414eed6de410f1d4a8564 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Fri, 12 Feb 2021 00:48:19 -0500 Subject: [PATCH 3/3] DOC: add notes to reference draw methods The full artist tree needs to be walked in `draw` to ensure that deferred work is done. --- lib/matplotlib/backend_bases.py | 9 ++++++++- lib/matplotlib/backends/backend_pdf.py | 1 + lib/matplotlib/backends/backend_svg.py | 1 + lib/matplotlib/backends/backend_template.py | 9 ++++++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py index d6269e24a703..33831c4ba889 100644 --- a/lib/matplotlib/backend_bases.py +++ b/lib/matplotlib/backend_bases.py @@ -2030,7 +2030,14 @@ def release_mouse(self, ax): self.mouse_grabber = None def draw(self, *args, **kwargs): - """Render the `.Figure`.""" + """ + Render the `.Figure`. + + It is important that this method actually walk the artist tree + even if not output is produced because this will trigger + deferred work (like computing limits auto-limits and tick + values) that users may want access to before saving to disk. + """ def draw_idle(self, *args, **kwargs): """ diff --git a/lib/matplotlib/backends/backend_pdf.py b/lib/matplotlib/backends/backend_pdf.py index 2f5fb2bd990f..42fa04b2fd47 100644 --- a/lib/matplotlib/backends/backend_pdf.py +++ b/lib/matplotlib/backends/backend_pdf.py @@ -2734,6 +2734,7 @@ def draw(self): _no_output_draw(self.figure) return super().draw() + FigureManagerPdf = FigureManagerBase diff --git a/lib/matplotlib/backends/backend_svg.py b/lib/matplotlib/backends/backend_svg.py index 401d7d91268a..21a853693b1c 100644 --- a/lib/matplotlib/backends/backend_svg.py +++ b/lib/matplotlib/backends/backend_svg.py @@ -1367,6 +1367,7 @@ def draw(self): _no_output_draw(self.figure) return super().draw() + FigureManagerSVG = FigureManagerBase diff --git a/lib/matplotlib/backends/backend_template.py b/lib/matplotlib/backends/backend_template.py index 5262a01fb6af..7ed017a0c6eb 100644 --- a/lib/matplotlib/backends/backend_template.py +++ b/lib/matplotlib/backends/backend_template.py @@ -191,7 +191,14 @@ class methods button_press_event, button_release_event, """ def draw(self): - """Draw the figure using the renderer.""" + """ + Draw the figure using the renderer. + + It is important that this method actually walk the artist tree + even if not output is produced because this will trigger + deferred work (like computing limits auto-limits and tick + values) that users may want access to before saving to disk. + """ renderer = RendererTemplate(self.figure.dpi) self.figure.draw(renderer)