DOC: Suppress warnings in visualization.rst by sinhrks · Pull Request #8877 · pandas-dev/pandas (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Closes #8234. Sorry to take a long.

I understand there are 2 warnings:

1. Warning in /Users/sin/Documents/Git/pandas/doc/source/visualization.rst at block ending on line 1275
   This is raised from mpl as plotting empty axes for demonstration purpose. Thus simply suppressed.
2. Warning in /Users/sin/Documents/Git/pandas/doc/source/visualization.rst at block ending on line 1312
   This is a warning displayed when passing multiple axes. Added explanations.

Choose a reason for hiding this comment

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

Is this change so you see better the empty axes?

Choose a reason for hiding this comment

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

The explanation on line 1293 is now no longer correct.

Choose a reason for hiding this comment

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

Yes, fixed the text and example.

just the tests, it would be good to take a look at that as well

@sinhrks can you see if you can fix the test warnings as well?

bumbed to 0.16, as you don't visually see it in the docs (warning is not shown by sphinx), so no blocker

Sure, will fix tests also.

Updated, and prepared a separate fix #9278 for a program change.

Just looking at the travis log, I still see some warnings. Eg (in the log of the third job: https://travis-ci.org/pydata/pandas/jobs/48128130):

..................S.................................................../home/travis/build/pydata/pandas/pandas/tools/plotting.py:3225: UserWarning: To output multiple subplots, the figure containing the passed axes is being cleared

"is being cleared", UserWarning)

........../home/travis/build/pydata/pandas/pandas/tools/plotting.py:3208: UserWarning: When passing multiple axes, sharex and sharey are ignored.These settings must be specified when creating axes

"These settings must be specified when creating axes", UserWarning)

./home/travis/miniconda/envs/pandas/lib/python2.7/site-packages/matplotlib/axes.py:4747: UserWarning: No labeled objects found. Use label='...' kwarg on individual plots.

warnings.warn("No labeled objects found. "

It is not clear to me if you can see which test triggered the warnings.

Choose a reason for hiding this comment

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

Why is the 'catch warnings' needed here? I thought that explicitly stating sharex=False, sharey=False (as you do in this test) is enough to suppress the warning? (that is what you added in the docs?)

Choose a reason for hiding this comment

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

Required, because the test specifies layout keyword which can't be effective when ax keyword is passed. I understand these tests was added in #8297, and intend to test layout is ignored properly.

@TomAugspurger ?

Choose a reason for hiding this comment

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

ah, that makes sense. Maybe explicitely test that the warning is triggered instead of ignoring it?

Choose a reason for hiding this comment

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

@sinhrks that sounds about right, but I could have added both the axes and layout kws without realizing that the layout would be ignored.

Choose a reason for hiding this comment

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

Was this causing warnings as well? I think I've seen warnings about trying to set the backend twice...

Choose a reason for hiding this comment

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

Yes it did, thus removed. Do you know the reason why these are added originally?

UserWarning: This call to matplotlib.use() has no effect because the the backend has already been chosen;
matplotlib.use() must be called *before* pylab, matplotlib.pyplot, or matplotlib.backends is 

Choose a reason for hiding this comment

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

The reason it causes a warning is because it is called after import matplotlib.pyplot as plt.
I was looking at the test_graphics, and there they also set the matplotlib backend, but in the test to skip it (https://github.com/sinhrks/pandas/blob/plot_doc/pandas/util/testing.py#L180). So you can also put it in the _skip_if_mpl_not_installed here. Only you have to import matplotlib in that test, and not matplotlib.pyplot

Choose a reason for hiding this comment

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

There is _skip_if_mpl_not_installed already. And I think matplotlib.pyplot must be installed here, because continuous test case accesses to pyplot.

So current fix (remove problematic part) looks OK if there is no specific reason to keep mpl.use.

Choose a reason for hiding this comment

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

I mean you can put the line in the existing _skip_if_mpl_not_installed. So adapt _skip_if_mpl_not_installed to import matplotlib and call mpl.use('Agg'), and then import pyplot here in the testµµ

I also don't exactly know why it is needed (maybe to run the tests where no graphical toolkit is avaliable?), but since all existing tests for plotting do this, I would keep it that way.

Choose a reason for hiding this comment

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

@jorisvandenbossche Understood what you mean. How about moving them to test_graphics.py to simplify.

Choose a reason for hiding this comment

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

Euhm, I would keep it here, as it really tests the plotting method of the GroupBy object. But maybe you could see if you can eg put the _skip_if_mpl_not_installed in util.testing for reuse.

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Ah I didn't know you had already a TestDataFrameGroupByPlots in test_graphics. Yep, certainly OK then to move the tests there.

But is seems a lot of the tests in TestDataFrameGroupByPlots don't really belong there (as it are just Series.plot tests)

Choose a reason for hiding this comment

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

Yeah, better to re-organize test_graphics. I'll issue separate one.

OK. I've only been able to give it a quick glance, but from that everything looks fine.

Regarding my comment above (#8877 (comment)) the No labeled objects found. Use label='...' kwarg on individual plots. is still present in the tests, but this is more a leftover not addressed in #9278, so see if you can look at it

Yes, will do. I originally misunderstood you're referring to different line.

@sinhrks This is ready to merge for you?

In any case, thanks for your endurance, this has been a long one ..

jorisvandenbossche added a commit that referenced this pull request

Feb 12, 2015

DOC: Suppress warnings in visualization.rst

Thanks!
Maybe then rebase #9464 to be sure

Labels