FIX/API: `fig.canvas.draw` always updates internal state by tacaswell · Pull Request #18408 · matplotlib/matplotlib · GitHub

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

I could be convinced that this should actually be backported for 3.3.2

I'm mixed if these needs an API change note or not.

PR Summary

PR Checklist

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (run flake8 on changed files to check).
Conforms to Matplotlib style conventions (install flake8-docstrings and pydocstyle<4 and run flake8 --docstring-convention=all).

I guess the superclass does nothing? It's a bit weird to do a no-op draw, and then go to the super's draw (which, from just looking at this one spot, could conceivably do anything.)

In 99.9% of cases, the super-call will do nothing, but I do not want to assume that no one has done some (ill-advised?) multiple inheritance of our classes and was relying on hitting their draw.

That said, I think the name _no_op_draw is bad because it is doing an operation (updating our internal state), just not producing any output.

This needs a rebase, and its corresponding issue is release critical, so marking the same.

rebased.

Perhaps the docstrings in backend_bases and backend_template should be updated to tell third-party implementers that they need to ensure that draw() walks the artist tree?

Minor comments, but basically good.

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 matplotlib#18407

Move the marks to skip if various latex installs are missing to be visible on other modules.

The full artist tree needs to be walked in `draw` to ensure that deferred work is done.

tacaswell added this to the v3.4.0 milestone Sep 4, 2020

QuLogic reviewed Sep 10, 2020

View reviewed changes

tacaswell force-pushed the fix_pgf_draw branch 2 times, most recently from 68fd2fe to 23a306f Compare September 23, 2020 15:33

tacaswell mentioned this pull request Oct 14, 2020

ticklabels empty when not interactive #6103

Closed

tacaswell force-pushed the fix_pgf_draw branch from 23a306f to c078356 Compare October 16, 2020 22:36

QuLogic added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. status: needs rebase labels Jan 21, 2021

tacaswell force-pushed the fix_pgf_draw branch from c078356 to 81e3c11 Compare January 22, 2021 18:43

tacaswell removed the status: needs rebase label Jan 22, 2021

jklymak reviewed Jan 24, 2021

View reviewed changes

Comment thread lib/matplotlib/testing/__init__.py

jklymak added the status: needs revision label Jan 27, 2021

tacaswell force-pushed the fix_pgf_draw branch 4 times, most recently from 76f95f7 to 3692ded Compare February 2, 2021 20:04

tacaswell removed the status: needs revision label Feb 3, 2021

QuLogic reviewed Feb 9, 2021

View reviewed changes

Comment thread lib/matplotlib/backends/backend_pdf.py Outdated

Comment thread lib/matplotlib/tests/test_backend_pgf.py

Comment thread lib/matplotlib/tests/test_backend_pgf.py Outdated

Comment thread lib/matplotlib/tests/test_backend_bases.py Outdated

QuLogic added the status: needs rebase label Feb 10, 2021

tacaswell force-pushed the fix_pgf_draw branch from 3692ded to 460ff37 Compare February 11, 2021 05:13

anntzer reviewed Feb 11, 2021

View reviewed changes

Comment thread lib/matplotlib/backends/backend_pdf.py

anntzer approved these changes Feb 11, 2021

View reviewed changes

QuLogic removed the status: needs rebase label Feb 12, 2021

QuLogic approved these changes Feb 12, 2021

View reviewed changes

tacaswell added 3 commits February 15, 2021 20:25

MNT: hoist latex helpers from pgf tests to matplotlib.testing

1173770

Move the marks to skip if various latex installs are missing to be visible on other modules.

DOC: add notes to reference draw methods

22e84f6

The full artist tree needs to be walked in `draw` to ensure that deferred work is done.

tacaswell force-pushed the fix_pgf_draw branch from 90efb61 to 22e84f6 Compare February 16, 2021 01:26

QuLogic merged commit 73b5101 into matplotlib:master Feb 17, 2021

tacaswell deleted the fix_pgf_draw branch February 17, 2021 02:41

This was referenced Feb 17, 2021

Request: for non-interactive backends make fig.canvas.draw() force the render #16558

Closed

WIP: for non-interactive backends make fig.canvas.draw() force the render #16573

Closed

tacaswell mentioned this pull request Mar 28, 2021

ENH: use canvas renderer in draw #19765

Closed

7 tasks

tacaswell mentioned this pull request Jun 1, 2023

canvas.draw() does not render tick labels with pdf backend #13513

Closed

FIX/API: `fig.canvas.draw` always updates internal state by tacaswell · Pull Request #18408 · matplotlib/matplotlib · GitHub

Repository navigation

FIX/API: fig.canvas.draw always updates internal state - #18408

FIX/API: fig.canvas.draw always updates internal state#18408
QuLogic merged 3 commits into
matplotlib:masterfrom
tacaswell:fix_pgf_draw

tacaswell commented Sep 4, 2020

Uh oh!

QuLogic Sep 10, 2020

Uh oh!

tacaswell Sep 23, 2020

Uh oh!

QuLogic commented Jan 21, 2021

Uh oh!

tacaswell commented Jan 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anntzer commented Feb 11, 2021

Uh oh!

anntzer left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

+                  def draw(self, *args, **kwargs):
+                      _no_op_draw(self.figure)
+                      return super().draw(*args, **kwargs)

FIX/API: `fig.canvas.draw` always updates internal state by tacaswell · Pull Request #18408 · matplotlib/matplotlib · GitHub

Repository navigation

FIX/API: fig.canvas.draw always updates internal state - #18408

FIX/API: fig.canvas.draw always updates internal state#18408QuLogic merged 3 commits intomatplotlib:mastermatplotlib/matplotlib:masterfrom tacaswell:fix_pgf_drawtacaswell/matplotlib:fix_pgf_drawCopy head branch name to clipboard

Conversation

tacaswell commented Sep 4, 2020

PR Summary

PR Checklist

Uh oh!

QuLogic Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

tacaswell Sep 23, 2020

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Jan 21, 2021

Uh oh!

tacaswell commented Jan 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anntzer commented Feb 11, 2021

Uh oh!

anntzer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

FIX/API: fig.canvas.draw always updates internal state#18408
QuLogic merged 3 commits into
matplotlib:masterfrom
tacaswell:fix_pgf_draw