FIX/API: fig.canvas.draw always updates internal state
#18408
Conversation
| def draw(self, *args, **kwargs): | ||
| _no_op_draw(self.figure) | ||
| return super().draw(*args, **kwargs) |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
68fd2fe to
23a306f
Compare
23a306f to
c078356
Compare
|
This needs a rebase, and its corresponding issue is release critical, so marking the same. |
c078356 to
81e3c11
Compare
|
rebased. |
76f95f7 to
3692ded
Compare
3692ded to
460ff37
Compare
|
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? |
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.
90efb61 to
22e84f6
Compare
Previously the non-interactive backends, other than Agg, did not
define
drawmethods 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
pytestpasses).flake8on changed files to check).flake8-docstringsandpydocstyle<4and runflake8 --docstring-convention=all).