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

Vendor test decorator from MPL? #2020

Closed
oscargus opened this issue Mar 24, 2022 · 4 comments · Fixed by #2023
Closed

Vendor test decorator from MPL? #2020

oscargus opened this issue Mar 24, 2022 · 4 comments · Fixed by #2023
Milestone

Comments

@oscargus
Copy link

Description

Matplotlib may deprecate the @cleanup test decorator matplotlib/matplotlib#22697 as it is no longer used in the Matplotlib code base. It was noted that Cartopy uses it, so my question now is where it should be vendored. To me it seems like creating a new file either testing.py or decorators.py may be options, possibly in the mpl directory. Or just add it to utils.py.

Any preferences?

(I can provide a PR, just in case.)

@dopplershift
Copy link
Contributor

dopplershift commented Mar 24, 2022

So it looks like @cleanup

  1. Sets style (we never use and can handle with pytest-mpl)
  2. Catches warnings
  3. Resets the unit registry
  4. Resets rcparams

(Did I miss anything?)

I don't know that we need any of that, except maybe the warning handling (which actually comes from the stdlib). So I'm wondering if we could just completely remove all uses?

@greglucas
Copy link
Contributor

I agree with removing the cleanup decorator if possible. It works for me locally at least, see #2023

@QuLogic
Copy link
Member

QuLogic commented Mar 25, 2022

I don't know that we need any of that, except maybe the warning handling (which actually comes from the stdlib). So I'm wondering if we could just completely remove all uses?

We don't need this either; it's caught by pytest.

@oscargus
Copy link
Author

Thanks all! Seems like it can work without it then. (The test failures in #2023 seems unrelated, so far.)

(This would then go in MPL 3.6, which is quite some time ahead, so should be plenty of time to get a Cartopy release out, if required. I guess all of you are at least as updated as I am when it comes to MPL releases, but in case someone else happens to see this.)

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 a pull request may close this issue.

4 participants