Skip to content

TST: fix warnings on multiple subplots #37274

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Nov 9, 2020
Merged

TST: fix warnings on multiple subplots #37274

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0ac41d1
TST: fix warnings on multiple subplots
ivanovmg Oct 20, 2020
bdb7001
Merge branch 'master' into warnings/matplotlib-subplots
ivanovmg Oct 23, 2020
66a5923
REF: rename to _check_plot_default_axes_works
ivanovmg Oct 29, 2020
8ab2bfd
Merge branch 'master' into warnings/matplotlib-subplots
ivanovmg Nov 4, 2020
ee1d941
REF: refactor using default_axes kwarg
ivanovmg Nov 9, 2020
d33322e
DOC: add docstring to _check_plot_works
ivanovmg Nov 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions pandas/tests/plotting/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,43 @@ def _unpack_cycler(self, rcParams, field="color"):
return [v[field] for v in rcParams["axes.prop_cycle"]]


def _check_single_plot_works(f, filterwarnings="always", **kwargs):
"""
Similar to _check_plot_works, but this one plots at the default axes,
without creating subplots.
"""
import matplotlib.pyplot as plt

ret = None
with warnings.catch_warnings():
warnings.simplefilter(filterwarnings)
try:
try:
fig = kwargs["figure"]
except KeyError:
fig = plt.gcf()

plt.clf()

ret = f(**kwargs)

tm.assert_is_valid_plot_return_object(ret)

with tm.ensure_clean(return_filelike=True) as path:
plt.savefig(path)
finally:
tm.close(fig)

return ret


def _check_plot_works(f, filterwarnings="always", **kwargs):
# Should bootstrap plot be removed from here to a separate function?
# Like _check_bootstrap_plot_works?
# And what if we do not create subplot(211)?
# This will save us a trouble with handling warnings,
# when figure internally creating subplots (like df.hist)
# is forced to be plotted in a canvas with subplots already created.
import matplotlib.pyplot as plt

ret = None
Expand Down
29 changes: 20 additions & 9 deletions pandas/tests/plotting/test_hist_method.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@

from pandas import DataFrame, Index, Series, to_datetime
import pandas._testing as tm
from pandas.tests.plotting.common import TestPlotBase, _check_plot_works
from pandas.tests.plotting.common import (
TestPlotBase,
_check_plot_works,
_check_single_plot_works,
)


@td.skip_if_no_mpl
Expand Down Expand Up @@ -138,7 +142,7 @@ def test_hist_with_legend(self, by, expected_axes_num, expected_layout):
s = Series(np.random.randn(30), index=index, name="a")
s.index.name = "b"

axes = _check_plot_works(s.hist, legend=True, by=by)
axes = _check_single_plot_works(s.hist, legend=True, by=by)
self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout)
self._check_legend_labels(axes, "a")

Expand Down Expand Up @@ -318,7 +322,7 @@ def test_tight_layout(self):
dtype=np.int64,
)
)
_check_plot_works(df.hist)
_check_single_plot_works(df.hist)
self.plt.tight_layout()

tm.close()
Expand All @@ -331,7 +335,7 @@ def test_hist_subplot_xrot(self):
"animal": ["pig", "rabbit", "pig", "pig", "rabbit"],
}
)
axes = _check_plot_works(
axes = _check_single_plot_works(
df.hist,
filterwarnings="always",
column="length",
Expand Down Expand Up @@ -360,10 +364,16 @@ def test_hist_column_order_unchanged(self, column, expected):
index=["pig", "rabbit", "duck", "chicken", "horse"],
)

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

assert result == expected
with tm.assert_produces_warning(UserWarning):
# _check_plot_works creates subplots inside,
# meanwhile df.hist here creates subplots itself for each column.
# It leads to warnings like this:
# 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))
Copy link
Member

@charlesdong1991 charlesdong1991 Oct 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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 )

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

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

Copy link
Member Author

@ivanovmg ivanovmg Oct 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

result = [axes[0, i].get_title() for i in range(3)]
assert result == expected

@pytest.mark.parametrize("by", [None, "c"])
@pytest.mark.parametrize("column", [None, "b"])
Expand All @@ -378,7 +388,8 @@ def test_hist_with_legend(self, by, column):
index = Index(15 * ["1"] + 15 * ["2"], name="c")
df = DataFrame(np.random.randn(30, 2), index=index, columns=["a", "b"])

axes = _check_plot_works(df.hist, legend=True, by=by, column=column)
axes = _check_single_plot_works(df.hist, legend=True, by=by, column=column)

self._check_axes_shape(axes, axes_num=expected_axes_num, layout=expected_layout)
if by is None and column is None:
axes = axes[0]
Expand Down