Backend class for better code reuse between backend modules #8773
Conversation
bf03f1e to
544823a
Compare
|
How does this interact with #4143 ? |
| FigureManager = FigureManagerPgf | ||
|
|
||
|
|
||
| def draw_if_interactive(): |
There was a problem hiding this comment.
Should this be a class method on the preceding class?
There was a problem hiding this comment.
Either way is technically fine (this def will just override the one exported by the decorator) but actually I can just leave trigger_manager_draw as None and this way we'll get a do-nothing draw_if_interactive.
|
Given how disruptive this change could be, I am surprisingly 👍 on this 😄 . This is a very elegant solution to a problem that has been bugging me for years. |
|
And to answer my own question, I think this is mostly orthogonal. |
|
Re #4143 (MEP27): this doesn't really change anything, MEP27 can still go in (modulo conflicts, but they're already there anyways) (and I don't really have an opinion regarding MEP27). This is really just a way to 1. use some private "templates" to define helper functions and 2. do |
811b2d0 to
05aae1e
Compare
| # each draw command. Instead, we mark the figure as invalid, so that it | ||
| # will be redrawn as soon as the event loop resumes via PyOS_InputHook. | ||
| # This function should be called after each draw event, even if | ||
| # matplotlib is not running interactively. |
There was a problem hiding this comment.
This comment, taken from the original, has me puzzled. It seems to be contradicted by the code, which does nothing unless is_interactive() returns True. This is not changed by your refactor, so if the comment is misleading now, then it was also misleading before; or I am just missing its point in both cases.
There was a problem hiding this comment.
I'm going to leave it as it is for now...
|
I certainly welcome the reduced duplication (which always bothered me also) and reduced LOC. And the removal of some cruft, like the import of ctypes! |
795c171 to
1c3da14
Compare
a422dca to
2254671
Compare
|
done |
|
I am going to give this one more read through before pushing the merge button myself, but happy if someone else also reviews this and merges. |
|
Rebased. |
This is the second part of #8771, split out for simpler review (note that it also includes the first part, which is the object of #8772; thus, I would suggest specifically looking at the diff of the last commit).
Currently, backend modules need to define a number of classes (
FigureCanvas,FigureManager) and functions (new_figure_manager,new_figure_manager_given_figure,show,draw_if_interactive) with specified names, which leads to some serious copy-pasting between the modules, as well as seemingly unused imports (see e.g. "not used" comment in backend_qt5agg.py). Instead, use a class-based approach: the_Backendclass can be subclassed to define these functions (and provides some templates for them); then a special decorator re-exports the attributes and methods of the_Backendclass to the backend-defining module.