Skip to content
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

TST: Prepare for pytest 9 #28195

Merged
merged 1 commit into from
May 9, 2024
Merged

TST: Prepare for pytest 9 #28195

merged 1 commit into from
May 9, 2024

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented May 9, 2024

PR summary

The current version of pytest is warning that using importorskip to catch ImportError will start being ignored (and thus raising) with pytest 9.

Fortunately, in all cases, we don't need these calls, as they are:

  • already checked for ImportError at the top-level of the file
  • already checked by the backend switcher
  • not actually possible to fail importing

PR checklist

The current version of pytest is warning that using `importorskip` to
catch `ImportError` will start being ignored (and thus raising) with
pytest 9.

Fortunately, in all cases, we don't need these calls, as they are:

- already checked for `ImportError` at the top-level of the file
- already checked by the backend switcher
- not actually possible to fail importing
@QuLogic QuLogic added this to the v3.9.1 milestone May 9, 2024
test_backend = pytest.importorskip(
f'matplotlib.backends.backend_{backend}'
)
test_backend = importlib.import_module(f'matplotlib.backends.backend_{backend}')
Copy link
Member Author

Choose a reason for hiding this comment

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

backend here is svg, ps, pdf, or pgf, which are not expected to fail (maybe they used to when Pillow was optional?)

Copy link
Member

Choose a reason for hiding this comment

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

maybe crossed streams with the mark on the pgf parameter?

@@ -3,9 +3,6 @@
import pytest


pytest.importorskip("matplotlib.backends.backend_gtk3agg")


@pytest.mark.backend("gtk3agg", skip_on_importerror=True)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only test, so the backend switch covers the import check.

qt_compat = pytest.importorskip('matplotlib.backends.qt_compat')
QtCore = qt_compat.QtCore

from matplotlib.backends.qt_compat import QtCore
Copy link
Member Author

@QuLogic QuLogic May 9, 2024

Choose a reason for hiding this comment

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

qt_compat is checked above on line 18, and the whole module is skipped.

@tacaswell
Copy link
Member

importorskip to catch ImportError will start being ignored (and thus raising) with pytest 9.

Doesn't that defeat the purpose of importorskip ?!

@QuLogic
Copy link
Member Author

QuLogic commented May 9, 2024

importorskip to catch ImportError will start being ignored (and thus raising) with pytest 9.

Doesn't that defeat the purpose of importorskip ?!

They plan to only catch ModuleNotFoundError by default in the future: https://docs.pytest.org/en/8.2.x/deprecations.html#pytest-importorskip-default-behavior-regarding-importerror

@tacaswell
Copy link
Member

Ah ha! I completely missed the addition of ModuleNotFound (which was in 3.6) so this makes sense.

@tacaswell tacaswell merged commit eb17273 into matplotlib:main May 9, 2024
41 of 43 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request May 9, 2024
@QuLogic QuLogic deleted the pytest9 branch May 9, 2024 21:03
timhoffm added a commit that referenced this pull request May 10, 2024
…195-on-v3.9.x

Backport PR #28195 on branch v3.9.x (TST: Prepare for pytest 9)
@QuLogic QuLogic modified the milestones: v3.9.1, v3.9.0 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants