-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: ensure switching the backend installs repl hook #23057
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The importance of the lack of the trailing comma is too subtle, went with a ';'.join(...)
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appears to fix using the example in #23042; not sure I want to dig into pylab to see why.
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't really need this, nor the stdout/stderr capture (pytest does it for you), nor universal_newlines, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
pyplot
so 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_displayhook
had never been called.PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).I'm still thinking about how to test this.