Skip to content

Cleanups to webagg & friends. #19127

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
Dec 16, 2020
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Dec 16, 2020

FigureCanvasWebAgg and FigureCanvasNbAgg empty subclasses of
FigureCanvasWebAggCore. They only differed in that WebAggCore did not
define the (common) timer class, which is easily fixed (but the
TimerTornado definition needs to be moved above the
FigureCanvasWegAggCore definition), and that WebAgg explicitly redefined
show to use the module's show, whereas WebAggCore's show uses
pyplot.show... but pyplot.show is defined as calling the backend
module's show, so it comes down to the same.

Still it's useful for them to be subclasses rather than straight
aliases, to keep the possibility of a canvas->manager mapping (#18854).

No need to explicitly mark WebAggCore as supports_blit, as that's
autodetected now (one can easily check that
FigureCanvasWebAggCore.supports_blit is still True).

Saving pgf to a BytesIO actually works fine nowadays, and can indeed
by manually tested on WebAgg.

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • 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).

FigureCanvasWebAgg and FigureCanvasNbAgg empty subclasses of
FigureCanvasWebAggCore.  They only differed in that WebAggCore did not
define the (common) timer class, which is easily fixed (but the
TimerTornado definition needs to be moved above the
FigureCanvasWegAggCore definition), and that WebAgg explicitly redefined
`show` to use the module's `show`, whereas WebAggCore's `show` uses
`pyplot.show`... but `pyplot.show` is defined as calling the backend
module's `show`, so it comes down to the same.

Still it's useful for them to be subclasses rather than straight
aliases, to keep the possibility of a canvas->manager mapping.

No need to explicitly mark WebAggCore as `supports_blit`, as that's
autodetected now (one can easily check that
`FigureCanvasWebAggCore.supports_blit` is still True).

Saving `pgf` to a BytesIO actually works fine nowadays, and can indeed
by manually tested on WebAgg.
@tacaswell tacaswell added this to the v3.4.0 milestone Dec 16, 2020
@jklymak
Copy link
Member

jklymak commented Dec 16, 2020

What is the test for this, since its largely untested by CI?

@anntzer
Copy link
Contributor Author

anntzer commented Dec 16, 2020

test_backends_interactive.py::test_webagg should cover a good part of it? In fact looking at https://codecov.io/gh/matplotlib/matplotlib/commit/25ee876375a1d98f9f925348a2578d40cc75cd0b the sole source of non-coverage is the fact that some of TimerTornado is untested, but I just shifted the whole implementation verbatim upwards in the source file...

@dopplershift dopplershift merged commit 39737b8 into matplotlib:master Dec 16, 2020
@anntzer anntzer deleted the webagg branch December 16, 2020 19:21
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