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: fix some mpl warnings #38574

Merged
merged 6 commits into from
Dec 22, 2020

Conversation

mzeitlin11
Copy link
Member

  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

This change fixes UserWarning being emitted in tests calling autocorrelation_plot, andrews_curves, and radviz.

UserWarning was caused by calling plt.gca(xlim=..., ylim=...) on a potentially already existing axis, so matplotlib warns about this forcing new axis creation. Solution was just to explicitly use set_xlim and set_ylim instead.

Does it make sense to also add to the relevant tests an assertion that no UserWarning is emitted to guarantee the new behavior?

Copy link
Member

@ivanovmg ivanovmg left a comment

Choose a reason for hiding this comment

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

Please see the comment below.
I agree that addition of the tests is necessary.

Comment on lines 148 to 149
ax.set_xlim([-1, 1])
ax.set_ylim([-1, 1])
Copy link
Member

Choose a reason for hiding this comment

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

I guess you can even use ax.set_xlim(-1, 1), without extra brackets.
The same for other cases as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@jreback jreback added Testing pandas testing functions or related to the test suite Visualization plotting labels Dec 21, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

yeah pls add a tests that asssert_produces_warning(None) and fails on master but passes here

@mzeitlin11
Copy link
Member Author

Will add tests once #38620 is figured out

@jreback
Copy link
Contributor

jreback commented Dec 21, 2020

Will add tests once #38620 is figured out

hmm tm.assert_produces_warning(None) should just work

@mzeitlin11
Copy link
Member Author

Will add tests once #38620 is figured out

hmm tm.assert_produces_warning(None) should just work

It wasn't raising even when a warning was returned, with examples of this given in the issue. So there are currently no failures in master when wrapping these plotting calls with tm.assert_produces_warning(None), even though a warning is returned

@mzeitlin11
Copy link
Member Author

Wrapped existing calls that give warnings on master to avoid adding more plotting calls. If splitting out new tests for this is preferred instead, please let me know

@jreback jreback added this to the 1.3 milestone Dec 22, 2020
@jreback
Copy link
Contributor

jreback commented Dec 22, 2020

Wrapped existing calls that give warnings on master to avoid adding more plotting calls. If splitting out new tests for this is preferred instead, please let me know

this looks good; ping on green

@mzeitlin11
Copy link
Member Author

Green

@jreback jreback merged commit f3a9271 into pandas-dev:master Dec 22, 2020
@jreback
Copy link
Contributor

jreback commented Dec 22, 2020

thanks @mzeitlin11

@mzeitlin11 mzeitlin11 deleted the tst/avoid_some_mpl_warnings branch December 22, 2020 22:25
luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021
@simonjayhawkins
Copy link
Member

@meeseeksdev backport 1.2.x

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Apr 6, 2021
@simonjayhawkins simonjayhawkins modified the milestones: 1.3, 1.2.4 Apr 6, 2021
simonjayhawkins pushed a commit that referenced this pull request Apr 6, 2021
Co-authored-by: mzeitlin11 <37011898+mzeitlin11@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing pandas testing functions or related to the test suite Visualization plotting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants