NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

TST: Add test for _repr_html_#18501

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

Jump to bottom

Merged

QuLogic merged 4 commits intomatplotlib:masterfromlarsoner:test

Sep 22, 2020

Merged

TST: Add test for _repr_html_#18501

QuLogic merged 4 commits intomatplotlib:masterfromlarsoner:test

Sep 22, 2020

Conversation

Copy link

Contributor

larsoner commentedSep 16, 2020

PR Summary

I got some TemporaryDirectory ResourceWarning's in a sphinx-gallery-based doc builds with matplotlib animations, so I started trying to replicate them with some minimal code. I couldn't, but it looked like there weren't any explicit_repr_html_ tests in thetest_animation.py code, so I figured I'd open a PR to add some basic ones (these were my minimal examples that showed no problems at the mpl end).

PR Checklist

Has pytest style unit tests (andpytest passes).
IsFlake 8 compliant (runflake8 on changed files to check).
New features are documented, with examples if plot related.
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
Conforms to Matplotlib style conventions (installflake8-docstrings andpydocstyle<4 and runflake8 --docstring-convention=all).
New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

TST: Add test for _repr_html_

9cee283

QuLogic reviewed

Sep 17, 2020

View reviewed changes

Copy link

Member

QuLogic left a comment

Choose a reason for hiding this comment

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

You've got a few flake8 issues as well.

lib/matplotlib/tests/test_animation.py OutdatedShow resolvedHide resolved

MAINT: Unify

db52d85

dstansby added this to thev3.4.0 milestone

Sep 17, 2020

dstansby added the topic: testing label

Sep 17, 2020

dopplershift approved these changes

Sep 18, 2020

View reviewed changes

dopplershift added the topic: animation label

Sep 18, 2020

QuLogic reviewed

Sep 18, 2020

View reviewed changes

lib/matplotlib/tests/test_animation.py OutdatedShow resolvedHide resolved

STY: Rename fixture->klass

c3c32c8

QuLogic approved these changes

Sep 22, 2020

View reviewed changes

Copy link

Member

QuLogic left a comment

Choose a reason for hiding this comment

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

Everything looks good, but can you move theanim fixture earlier. I know it's not strictly required to define it before use, but it's nicer to keep fixtures together at the beginning (unless the file is long, but then it should probably be split up.)