Skip to content

Derive new_figure_manager from FigureCanvas.new_manager. #23090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 22, 2022

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented May 21, 2022

Followup to the introduction of FigureCanvas.new_manager (#22925): allow backend
modules to not define new_figure_manager anymore, in which case we
derive the needed function from FigureCanvas.new_manager. (In the
future, I plan to do the same with draw_if_interactive and show, so that
"a backend is just a module with a FigureCanvas class"; the advantage is
that the FigureCanvas subclass provided by the module can inherit
methods as needed from the parent class.)

For backcompat, the old codepath is maintained (and has priority).

To test this, manually alter backend_bases._Backend.export and remove
the new_figure_manager entry from the exported functions, which deletes
that global function from all of the builtin methods (actually, we'll
need a deprecation cycle), and check that pyplot still works fine. Also
tweak the testing machinery to restore the original backend even if the
backend was not switched via a pytest marker.

PR Summary

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@anntzer anntzer added this to the v3.6.0 milestone May 21, 2022
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

For backcompat, the old codepath is maintained (and has priority).

In the long run (some years) we don't want to have two code paths. We should explicitly discourage the old way. But maybe that should be done when show() and draw_if_interactive()are there as well.

This should be documented somewhere. Do we have technical documentation how a backend works?

@anntzer
Copy link
Contributor Author

anntzer commented May 21, 2022

My plan was to first make all the changes, and then only document the full new API. I don't really think there's a reference doc of what an old-style backend must provide (closest is probably backend_template) and the point of this series of PR was also to make that documentation easier to write.

I decided to start putting in these compat layers to show a bit the concrete end point before putting in more machinery for show and draw_if_interactive.

Also note that there's actually a third way to define backends currently, namely via the @_Backend decorator, which is private (and which I certainly intended as such), but used by ipympl, so that also needs to be properly deprecated... (or at least a patch needs to be written to help ipympl out of that, but again that should probably just wait for the new machinery to be completed first)

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Is it possible to test the logic?

@anntzer
Copy link
Contributor Author

anntzer commented May 22, 2022

Sure, found a way to do it which is a bit hacky, but heh...

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This seems fine to me, but maybe should get a double check from @tacaswell ?

@jklymak
Copy link
Member

jklymak commented May 22, 2022

BTW, seems if there will be a few of these they should get some label. I made one, but feel free to replace with something more appropriate or rename

Comment on lines 289 to 290
draw_if_interactive = getattr(backend_mod, "draw_if_interactive", None)
show = getattr(backend_mod, "show", None)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
draw_if_interactive = getattr(backend_mod, "draw_if_interactive", None)
show = getattr(backend_mod, "show", None)
# draw_if_interactive = getattr(backend_mod, "draw_if_interactive", None)
# show = getattr(backend_mod, "show", None)

These are not used yet so I would prefer to not define them yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

@tacaswell
Copy link
Member

which is private (and which I certainly intended as such), but used by ipympl,

Sorry, that is my fault.


"a backend is just a module with a FigureCanvas class";

I thought we were going for "A backend is a FigureCanvas class" ?

Followup to the introduction of FigureCanvas.new_manager: allow backend
modules to not define new_figure_manager anymore, in which case we
derive the needed function from FigureCanvas.new_manager.  (In the
future, I plan to do the same with draw_if_interactive and show, so that
"a backend is just a module with a FigureCanvas class"; the advantage is
that the FigureCanvas subclass provided by the module can inherit
methods as needed from the parent class.)

For backcompat, the old codepath is maintained (and has priority).

To test this, manually alter backend_bases._Backend.export and remove
the new_figure_manager entry from the exported functions, which deletes
that global function from all of the builtin methods (actually, we'll
need a deprecation cycle), and check that pyplot still works fine.  Also
tweak the testing machinery to restore the original backend even if the
backend was not switched via a pytest marker.
@anntzer
Copy link
Contributor Author

anntzer commented May 22, 2022

which is private (and which I certainly intended as such), but used by ipympl,

Sorry, that is my fault.

No worries, we'll figure something out...


"a backend is just a module with a FigureCanvas class";

I thought we were going for "A backend is a FigureCanvas class" ?

In practice you need to support the first form, so if what you actually have is just the class, you can just pass SimpleNamespace(FigureCanvas=my_class)?

@tacaswell tacaswell merged commit 8e34e70 into matplotlib:main May 22, 2022
@anntzer anntzer deleted the nfm branch May 23, 2022 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants