Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

TST: Fix tests with older versions of ipython#28205

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 1 commit intomatplotlib:mainfromQuLogic:old-ipython
May 10, 2024

Conversation

QuLogic
Copy link
Member

PR summary

I only went back as far as 7.0.0, as due to#16263, we probably don't want to be supporting all the way back to IPython 1.

I believe that this only affected the test, as on IPython <8.24, we use the code from IPython anyway. But I've pinned the tests to IPython 7 so we should see if everything's good.

Closes#28202

PR checklist

@QuLogic
Copy link
MemberAuthor

QuLogic commentedMay 10, 2024
edited
Loading

Ah, perhaps IPython 7.0.0 is a bit too old for Python 3.9 which was release Oct 5, 2020. Based on release dates, it seems like we should aim forat least IPython 7.19, released Oct 30, 2020? I didn't find any specific release notes about it except for "I’ve started to test on Python 3.9PR #12307 and fix some errors." in 7.15.

I only went back as far as 7.0.0, as due tomatplotlib#16263, we probably don'twant to be supporting all the way back to IPython 1.
@ianthomas23
Copy link
Member

I think we need to change the last line oflib/matplotlib/testing/__init__.py from

assertproc.stdout.strip()==f"Out[1]: '{expected_backend}'"

to something like

assertproc.stdout.strip().endswith(f"'{expected_backend}'")

as under some circumstances when I'm running old ipython but newish everything else I get (ignoring newlines)

Out[1]: Installed osx event loop hook. 'MacOSX'

@ianthomas23
Copy link
Member

Thanks for doing this, I didn't look very far back when I added these tests.

Locally I've tried this branch on Python 3.9 on a mac (to run both theqt andosx tests) with progressively older ipython, and it works (as in the tests pass) for ipython >= 7.3.0 but fails for ipython < 7.3.0. Ipython 7.3.0 was released in February 2019 so pre-dates Python 3.9 by a good year and a half but still seems to work with it. So officially covering >= 7.19 seems good to me as it is rule-based on the Python 3.9 release date.

Copy link
Member

@ianthomas23ianthomas23 left a comment

Choose a reason for hiding this comment

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

I'm approving as is but it would good to include theendswith change even though that line isn't yet changed by this PR and is only a problem when running tests locally rather than in CI.

For the failingtest_backend_inline.py::test_ipynb I would be inclined just to skip the test on Python 3.9. It relies on some of the Jupyter infrastructure working with a combination of old and new packages, and I don't think we should spend any time chasing that down when we have proved that IPython is OK here.

@tacaswelltacaswell merged commit1b7093d intomatplotlib:mainMay 10, 2024
42 of 44 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestMay 10, 2024
@QuLogic
Copy link
MemberAuthor

Err, that was a bit premature as tests weren't passing yet.

@QuLogicQuLogic deleted the old-ipython branchMay 10, 2024 18:18
QuLogic added a commit to QuLogic/matplotlib that referenced this pull requestMay 10, 2024
Make the changes suggested by@ianthomas23, and also mark the ipythontests as using their backend. While the backend is already checked foravailability at the top of the respective files, that only checkswhether it can be imported, not whether it can be set as the Matplotlibbackend. Adding the marker causes our pytest configuration to actuallycheck and skip the test if unavailable (e.g., on Linux without`(WAYLAND_)DISPLAY` set fails to set an interactive backend).
@QuLogicQuLogic mentioned this pull requestMay 10, 2024
1 task
@tacaswell
Copy link
Member

🤦🏻 Sorry, I only saw the coverage failure and missed the real failure.

QuLogic added a commit to meeseeksmachine/matplotlib that referenced this pull requestMay 10, 2024
Make the changes suggested by@ianthomas23, and also mark the ipythontests as using their backend. While the backend is already checked foravailability at the top of the respective files, that only checkswhether it can be imported, not whether it can be set as the Matplotlibbackend. Adding the marker causes our pytest configuration to actuallycheck and skip the test if unavailable (e.g., on Linux without`(WAYLAND_)DISPLAY` set fails to set an interactive backend).
ksunden added a commit that referenced this pull requestMay 13, 2024
ksunden added a commit that referenced this pull requestMay 13, 2024
…205-on-v3.9.xBackport PR#28205 on branch v3.9.x (TST: Fix tests with older versions of ipython)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ianthomas23ianthomas23ianthomas23 approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

[Bug]: Qt test_ipython fails on older ipython
3 participants
@QuLogic@ianthomas23@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp