Skip to content

Test whether it is viable to not close figures on backend switch. #15918

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Test whether it is viable to not close figures on backend switch. #15918

wants to merge 1 commit into from

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Dec 12, 2019 •
edited
Loading

DO NOT MERGE. The desired change would need a deprecation period, but
this is just to check on CI that the end state works.

Do not close("all") figures on (allowable) backend switches, e.g.
between qt5agg and qt5cairo, or qt5agg and notebook.

The NonGuiException message had to change, as the backend returned by
get_backend() may no longer match the actual canvas class.

xref #14471, #15913.

edit: looks like this is failing only on OSX + travis (but passing on OSX + azure...). Can anyone with access to a mac give it a look?

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer force-pushed the do-not-merge-nocloseall branch from 283017d to 48e5959 Compare December 12, 2019 11:46
@jklymak
Copy link
Member

jklymak commented Dec 12, 2019

Not quite sure what I'm supposed to test, but the following interactive session popped up two figures for me...

$ ipython
Python 3.7.3 | packaged by conda-forge | (default, Jul  1 2019, 14:38:56)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.8.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import matplotlib.pyplot as plt

In [2]: import matplotlib

In [3]: matplotlib.use('qt5agg')

In [4]: fig, ax = plt.subplots()

In [5]: plt.show(block=False)

In [6]: matplotlib.use('qt5cairo')

In [7]: fig, ax = plt.subplots()

In [8]: plt.show(block=False)

In [9]: fig.canvas
Out[9]: <matplotlib.backends.backend_qt5cairo.FigureCanvasQTCairo at 0x7ff1782e5948>

@jklymak
Copy link
Member

jklymak commented Dec 12, 2019

If I do

%matplotlib qt5
fig, ax = plt.subplots()
ax.plot(range(10))
plt.show(block=False)

in a notebook however, I get:

Warning: Cannot change to a different GUI toolkit: qt5. Using notebook instead.

@anntzer
Copy link
Contributor Author

anntzer commented Dec 12, 2019

Does running test_backends_interactive pass locally?

@jklymak
Copy link
Member

jklymak commented Dec 12, 2019 

$ py.test -vv --pyargs lib/matplotlib/tests/test_backends_interactive.py

============================================== test session starts ==============================================
platform darwin -- Python 3.7.3, pytest-5.2.1, py-1.8.0, pluggy-0.13.0 -- /Users/jklymak/anaconda3/envs/matplotlibdev/bin/python
cachedir: .pytest_cache
rootdir: /Users/jklymak/matplotlib, inifile: pytest.ini
collected 10 items

lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[gtk3agg] SKIPPED              [ 10%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[gtk3cairo] SKIPPED            [ 20%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[qt5agg0] PASSED               [ 30%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[qt5cairo0] SKIPPED            [ 40%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[qt5agg1] SKIPPED              [ 50%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[qt5cairo1] SKIPPED            [ 60%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[tkagg] PASSED                 [ 70%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[wx] SKIPPED                   [ 80%]
lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[wxagg] SKIPPED                [ 90%]
lib/matplotlib/tests/test_backends_interactive.py::test_webagg PASSED                                     [100%]

========================================= 3 passed, 7 skipped in 2.95s ==========================================

@jklymak
Copy link
Member

jklymak commented Dec 12, 2019

... not sure why the qt5cairo test skipped as that is installed.

@anntzer
Copy link
Contributor Author

anntzer commented Dec 12, 2019

Thanks, so now we (I) need to figure out why the same test is failing on travis...
I think the backend-switch prevention you see is something from ipython (or the notebook) and may need to be patched out there.

@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Jul 3, 2023
@anntzer anntzer marked this pull request as ready for review July 3, 2023 11:59
@anntzer anntzer force-pushed the do-not-merge-nocloseall branch from 48e5959 to c8ca4fc Compare July 3, 2023 11:59
@anntzer anntzer force-pushed the do-not-merge-nocloseall branch 2 times, most recently from df8f904 to bb05fb5 Compare July 3, 2023 17:00
@github-actions github-actions bot removed the status: inactive Marked by the “Stale” Github Action label Jul 5, 2023
@jklymak
Copy link
Member

jklymak commented Jul 6, 2023

@anntzer you rebased this, but its not clear what state this is in. I've moved to draft but feel free to move back and tell us what you need.

@jklymak jklymak marked this pull request as draft July 6, 2023 16:33
@anntzer
Copy link
Contributor Author

anntzer commented Jul 6, 2023

This still needs some work.

@anntzer anntzer force-pushed the do-not-merge-nocloseall branch from bb05fb5 to 7c56ecf Compare August 6, 2023 19:03
@anntzer
Test whether it is viable to not close figures on backend switch.
1048c67 
DO NOT MERGE.  The desired change would need a deprecation period, but
this is just to check on CI that the end state works.

Do not `close("all")` figures on (allowable) backend switches, e.g.
between qt5agg and qt5cairo, or qt5agg and notebook.

The NonGuiException message had to change, as the backend returned by
get_backend() may no longer match the actual canvas class.
@anntzer anntzer force-pushed the do-not-merge-nocloseall branch from 7c56ecf to 1048c67 Compare August 8, 2023 12:37
@anntzer anntzer closed this Aug 8, 2023
@anntzer anntzer deleted the do-not-merge-nocloseall branch August 8, 2023 20:49
@anntzer anntzer mentioned this pull request Aug 8, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anntzer @jklymak