FIX: ensure switching the backend installs repl hook#23057
FIX: ensure switching the backend installs repl hook#23057anntzer merged 2 commits intomatplotlib:mainfrom
Conversation
lib/matplotlib/tests/test_pyplot.py
Outdated
| "IPython", | ||
| "--pylab", | ||
| "-c", | ||
| "import matplotlib.pyplot as plt; assert plt._REPL_DISPLAYHOOK == plt._ReplDisplayHook.IPYTHON", # noqa |
There was a problem hiding this comment.
I would just implictly join the strings here to split this over two lines
[
sys.executable,
"-mIPython",
"--pylab",
"-c",
"import matplotlib.pyplot as plt; "
"assert plt._REPL_DISPLAYHOOK == plt._ReplDisplayHook.IPYTHON",
],There was a problem hiding this comment.
The importance of the lack of the trailing comma is too subtle, went with a ';'.join(...) instead.
closes matplotlib#23042 In matplotlib#22005 we change `pyplot` so that at import time we do not force a switch of the backend and install the repl displayhook. However, this meant that in some cases (primarily through `ipython --pylab`) to end up with a session where `install_repl_displayhook` had never been called. Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
timhoffm
left a comment
There was a problem hiding this comment.
This causes timeouts in the py39 and py310 azure builds.
We "just" need unreasonably long timeouts 🤷🏻 (was 5s, went to 60s). |
| "assert plt._REPL_DISPLAYHOOK == plt._ReplDisplayHook.IPYTHON", | ||
| )), | ||
| ], | ||
| env={**os.environ, "SOURCE_DATE_EPOCH": "0"}, |
There was a problem hiding this comment.
You don't really need this, nor the stdout/stderr capture (pytest does it for you), nor universal_newlines, I think?
anntzer
left a comment
There was a problem hiding this comment.
Minor nit on test, feel free to selfmerge with or without fixing.
|
I'll put the cleanup in a separate PR. |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
…lls repl hook Merge pull request matplotlib#23057 from tacaswell/fix_pylab (cherry picked from commit b31c5ae)
* Backport PR #23057: FIX: ensure switching the backend installs repl hook Merge pull request #23057 from tacaswell/fix_pylab (cherry picked from commit b31c5ae) * Merge pull request #23106 from anntzer/tpi MNT: Reuse subprocess_run_helper in test_pylab_integration. (cherry picked from commit 51624b6) * TST: Adjust to state tracking on 3.5.x branch The implementation of tracking if the repl displayhook is installed changed on main. Adjust the backported test to work with the old way. Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
PR Summary
closes #23042
In #22005 we change
pyplotso that at import time we do not force a switch ofthe backend and install the repl displayhook.
However, this meant that in some cases (primarily through
ipython --pylab) toend up with a session where
install_repl_displayhookhad never been called.PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).I'm still thinking about how to test this.