TST: fix warnings on multiple subplots #37274

ivanovmg · 2020-10-20T12:16:02Z

xref TST: suppress matplotlib warnings (or determine if they represent a bug) #37178 partially
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

This PR suggests a (partial) fix for such warnings, which take place when running test suite on pandas/tests/plotting.

UserWarning: To output multiple subplots, the figure containing the passed axes is being cleared

The problem is the following.
The function _check_plot_works creates subplot(211) if axes are not provided.
This is fine, when the plot to be created is a single-axes plot.
However, if the plot itself is to be plotted on multiple axes (for instance, df.hist would plot each column on a dedicated subplot, by default), then the warnings mentioned are emitted.

Two ways to handle it:

Find cases where plot is a multiaxes-plot and catch warnings there. This is fine, but in some parametrized tests the plots are either single-axes or multiple-axes. It makes it difficult to figure out without splitting the tests.
Change function _check_plot_works: do not create subplot(211).

I created _check_single_plot_works which does not create subplots and used it in the problematic cases.
I guess, that the name is not the best (I would prefer _check_plot_works), but I encourage those concerned to have a discussion here.

Please see comments in _check_plot_works (I will remove them once we come to the conclusion on the topic).

ivanovmg · 2020-10-20T12:42:31Z

Tests related to datetime failed in in Linux py38_locale. Should be unrelated to the topic.

jreback · 2020-10-20T23:23:25Z

looks fine to me. @charlesdong1991 @simonjayhawkins @WillAyd if any comments.

WillAyd · 2020-10-22T19:36:13Z

Can you merge master again? Is the Windows failure not related?

simonjayhawkins

I'm not familiar with the code here. Is there a solution without so much code duplication?

ivanovmg · 2020-10-23T12:16:37Z

I'm not familiar with the code here. Is there a solution without so much code duplication?

I can extract portions of the code from _check_plot_works, so that the duplication is reduced.
For that I will refactor _check_plot_works.
However, before doing so, it seems reasonable to figure out whether we need to split _check_plot_works into multiple functions (for example, check_single_plot_works, check_subplots_works, check_bootstrap_plot_works).

@charlesdong1991

simonjayhawkins · 2020-10-23T12:38:13Z

is adding an extra keyword argument to _check_plot_works not an option?

ivanovmg · 2020-10-23T13:30:11Z

is adding an extra keyword argument to _check_plot_works not an option?

I tried to implement a parameter force_subplots = True (by default create plots in two subplots), and get rid of _check_single_plot_works, but the logic is too complex (and too many levels of indentation).

ivanovmg · 2020-10-23T16:30:24Z

Can you merge master again? Is the Windows failure not related?

Tests on windows failed now because of internal CI problem.

charlesdong1991 · 2020-10-26T19:35:34Z

pandas/tests/plotting/test_hist_method.py

+            # UserWarning: To output multiple subplots,
+            # the figure containing the passed axes is being cleared
+            # Similar warnings were observed in GH #13188
+            axes = _check_plot_works(df.hist, column=column, layout=(1, 3))


emm, will _check_single_plot_works not raise warning here? seems the purpose of having _check_single_plot_works is to avoid creating subplots inside function?

Right, _check_single_plot_works does not raise the warning concerned, regarding figure being cleared.
In my opinion the need for _check_single_plot_works (or whatever the better name would be) is when the plotting method (like DataFrame.hist) creates subplots itself (may depend on the the options provided to the method).
The warning gets raised when the plotting method creates enough subplots required on its own, but we forcefully add subplots in _check_plot_works.

emm, okay. for DataFrame.hist, i think as long as it plots data from multiple columns, or a by is used, then will have subplots.

my main concern is in pandas.plot API where the implementation is different, and there is an argument called subplots, then do we also need to use _check_single_plot_works (or probably other name is better) when subplots=True for those test cases?

Instead of putting in-line comments under _check_plot_works function, maybe it's nicer to put few words about usecase/guideline of when to suggest to use which so that could help future contributors select the right function? how does this sound?

Instead of putting in-line comments under _check_plot_works function, maybe it's nicer to put few words about usecase/guideline of when to suggest to use which so that could help future contributors select the right function? how does this sound?

I actually thought of not using comments here and instead call _check_single_plot_works. This way there will be no warning raised.

axes = _check_single_plot_works(df.hist, column=column, layout=(1, 3)) result = [axes[0, i].get_title() for i in range(3)] assert result == expected

emm, okay. for DataFrame.hist, i think as long as it plots data from multiple columns, or a by is used, then will have subplots.

my main concern is in pandas.plot API where the implementation is different, and there is an argument called subplots, then do we also need to use _check_single_plot_works (or probably other name is better) when subplots=True for those test cases?

Right, is subplots=True, then it seems that we need to use _check_single_plot_works.
But I really don't like the name of this function )

This way there will be no warning raised.

yes, #37274 (comment) actually I was here wondering why not use _check_single_plot_works to remove warning

I actually thought of not using comments

sorry for my confusing comment, i meant to have one sentence to tell future users when to use _check_single_plot_works and when to use _check_plot_works in testing, something like Using _check_single_plot_works when plotting methods generate a subplot itself, if not, use _check_plot_works will be enough IMHO.

But I really don't like the name of this function

indeed, maybe _check_plot_with_subplots_works? or something more straightforward/explicit?

What about _check_plot_default_axes_works?
I replaced catching warning with the use of the new function as well.

@charlesdong1991

I do not like _check_plot_with_subplots_works either because it is not clear what the word subplots means here.
In fact original _check_plot_works creates subplots, so one can easily mix-up the meaning of the subplots term.

jreback · 2020-11-04T03:00:43Z

@ivanovmg can you merge master and update to comments

jreback · 2020-11-04T13:43:06Z

we need to combine these check_plot_works to a single function (add an arg to differentiate them)

ivanovmg · 2020-11-09T11:20:15Z

pandas/tests/plotting/common.py

+def _gen_default_plot(f, fig, **kwargs):
+    """
+    Create plot in a default way.
+    """
+    yield f(**kwargs)


Here fig is a dummy variable.
We have it here to ensure the same function signature with _get_two_subplots.

Alternatively, we can create empty figure in both generator functions and yield fig, f(**kwargs).

It seemed to me that yielding one argument rather than two is clearer, thus I did it the way it is now.

ivanovmg · 2020-11-09T11:23:37Z

is adding an extra keyword argument to _check_plot_works not an option?

I tried to implement a parameter force_subplots = True (by default create plots in two subplots), and get rid of _check_single_plot_works, but the logic is too complex (and too many levels of indentation).

I figured out how to implement it with an optional keyword argument.
Please have a look.
@jreback @charlesdong1991 @jreback

jreback · 2020-11-09T13:57:28Z

lgtm @charlesdong1991 @WillAyd if any comments

charlesdong1991 · 2020-11-09T15:30:59Z

would be nicer to put a sentence in docs to briefly explain the new added kwarg a bit

but also okay if merge ^^

ivanovmg · 2020-11-09T17:39:24Z

would be nicer to put a sentence in docs to briefly explain the new added kwarg a bit

but also okay if merge ^^

I added the docstring. Please take a look.

charlesdong1991 · 2020-11-09T18:13:43Z

great work! @ivanovmg

jreback · 2020-11-09T18:55:12Z

thanks @ivanovmg

TST: fix warnings on multiple subplots

0ac41d1

ivanovmg requested review from charlesdong1991 and jbrockmendel October 20, 2020 14:20

jreback added the Testing pandas testing functions or related to the test suite label Oct 20, 2020

jreback added this to the 1.2 milestone Oct 20, 2020

simonjayhawkins reviewed Oct 23, 2020

View reviewed changes

Merge branch 'master' into warnings/matplotlib-subplots

bdb7001

charlesdong1991 reviewed Oct 26, 2020

View reviewed changes

REF: rename to _check_plot_default_axes_works

66a5923

Merge branch 'master' into warnings/matplotlib-subplots

8ab2bfd

REF: refactor using default_axes kwarg

ee1d941

ivanovmg commented Nov 9, 2020

View reviewed changes

DOC: add docstring to _check_plot_works

d33322e

jreback merged commit c226fc5 into pandas-dev:master Nov 9, 2020

ivanovmg deleted the warnings/matplotlib-subplots branch November 10, 2020 04:45

ivanovmg mentioned this pull request Nov 10, 2020

TST: use default_axes=True where multiple plots #37734

Merged

5 tasks

jbrockmendel mentioned this pull request Nov 11, 2020

TST: suppress matplotlib warnings (or determine if they represent a bug) #37178

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: fix warnings on multiple subplots #37274

TST: fix warnings on multiple subplots #37274

ivanovmg commented Oct 20, 2020 •

edited by jreback

Loading

ivanovmg commented Oct 20, 2020

jreback commented Oct 20, 2020

WillAyd commented Oct 22, 2020

simonjayhawkins left a comment

ivanovmg commented Oct 23, 2020 •

edited

Loading

simonjayhawkins commented Oct 23, 2020

ivanovmg commented Oct 23, 2020

ivanovmg commented Oct 23, 2020

charlesdong1991 Oct 26, 2020 •

edited

Loading

ivanovmg Oct 26, 2020

charlesdong1991 Oct 26, 2020

charlesdong1991 Oct 26, 2020

ivanovmg Oct 28, 2020

ivanovmg Oct 28, 2020

charlesdong1991 Oct 29, 2020

charlesdong1991 Oct 29, 2020

ivanovmg Oct 29, 2020 •

edited

Loading

jreback commented Nov 4, 2020

jreback commented Nov 4, 2020

ivanovmg Nov 9, 2020

ivanovmg commented Nov 9, 2020

jreback commented Nov 9, 2020

charlesdong1991 commented Nov 9, 2020

ivanovmg commented Nov 9, 2020

charlesdong1991 commented Nov 9, 2020

jreback commented Nov 9, 2020

TST: fix warnings on multiple subplots #37274

TST: fix warnings on multiple subplots #37274

Conversation

ivanovmg commented Oct 20, 2020 • edited by jreback Loading

ivanovmg commented Oct 20, 2020

jreback commented Oct 20, 2020

WillAyd commented Oct 22, 2020

simonjayhawkins left a comment

Choose a reason for hiding this comment

ivanovmg commented Oct 23, 2020 • edited Loading

simonjayhawkins commented Oct 23, 2020

ivanovmg commented Oct 23, 2020

ivanovmg commented Oct 23, 2020

charlesdong1991 Oct 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg Oct 29, 2020 • edited Loading

Choose a reason for hiding this comment

jreback commented Nov 4, 2020

jreback commented Nov 4, 2020

Choose a reason for hiding this comment

ivanovmg commented Nov 9, 2020

jreback commented Nov 9, 2020

charlesdong1991 commented Nov 9, 2020

ivanovmg commented Nov 9, 2020

charlesdong1991 commented Nov 9, 2020

jreback commented Nov 9, 2020

ivanovmg commented Oct 20, 2020 •

edited by jreback

Loading

ivanovmg commented Oct 23, 2020 •

edited

Loading

charlesdong1991 Oct 26, 2020 •

edited

Loading

ivanovmg Oct 29, 2020 •

edited

Loading