FIX: fix cleanup warnings for errorbar timeseries #36982

ivanovmg · 2020-10-08T14:52:03Z

Currently in pandas/tests/plotting/tests_frame.py there are several warnings emitted.

pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_mpl2_color_cycle_str
  /workspaces/pandas/pandas/plotting/_matplotlib/style.py:64: MatplotlibDeprecationWarning: Support for uppercase single-letter colors is deprecated since Matplotlib 3.1 and will be removed in 3.3; please use lowercase instead.
    [conv.to_rgba(c) for c in colors]

pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_errorbar_plot
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_errorbar_plot
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_errorbar_plot
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_errorbar_timeseries
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_errorbar_timeseries
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_errorbar_timeseries
  /workspaces/pandas/pandas/plotting/_matplotlib/__init__.py:61: UserWarning: To output multiple subplots, the figure containing the passed axes is being cleared
    plot_obj.generate()

-- Docs: https://docs.pytest.org/en/stable/warnings.html

This PR handles warnings in test_errorbar_timeseries.
If this approach is considered reasonable by the reviewers, then I will do the same for test_errorbar_plot as well as other similar warnings among the plotting-related tests.

jreback · 2020-10-08T21:21:35Z

so the test itself was causing the warnings?

jreback · 2020-10-08T21:21:56Z

cc @charlesdong1991

ivanovmg · 2020-10-09T03:15:00Z

so the test itself was causing the warnings?

Yes, the test itself was causing the warning.

charlesdong1991 · 2020-10-09T06:51:13Z

this reminds me of this #13188 issue, the warning and the issue looks the same, should we use with tm.assert_produces_warning(UserWarning): to catch the part where subplots=True, something like this? and remove the catch_warning on the very top?

with tm.assert_produces_warning(UserWarning):
    axes = _check_plot_works(tdf.plot, kind=kind, yerr=tdf_err, subplots=True)
    self._check_has_errorbars(axes, xerr=0, yerr=1)

pandas/tests/plotting/test_frame.py

ivanovmg · 2020-10-09T07:05:23Z

this reminds me of this #13188 issue, the warning looks the same, should we use with tm.assert_produces_warning(UserWarning): to catch the part where subplots=True? or we should also use the same solution here to replace these warnings?

I did not use tm.assert_produces_warning because there is no way to match warning message this way.
I believe it is reasonable to match the message here.
What would others think?

charlesdong1991 · 2020-10-09T07:13:39Z

true, I would suggest to use tm.assert_produce_warning here to align with other existing similar issue, and see if we could add an arg for tm.assert_produce_warning to also match the warning message (I haven't looked at how this function is structured, not sure if it's easy or not to add such match), and then do a small refactor to match the warning message in these places.

does it sound good to you?

ivanovmg · 2020-10-09T07:16:44Z

true, I would suggest to use tm.assert_produce_warning here to align with other existing similar issue, and see if we could add an arg for tm.assert_produce_warning to also match the warning message (I haven't looked at how this function is structured, not sure if it's easy or not to add such match), and then do a small refactor to match the warning message in these places.

does it sound good to you?

Absolutely, I will refactor to tm.assert_produces_warning and add a comment what kind of warning message is expected.
Then in a follow-on PR I can take a look at the extension of tm.assert_produces_warning to match messages.
I believe that extension of tm.assert_produces_warning goes beyond the scope of the present PR.

charlesdong1991 · 2020-10-09T07:20:18Z

yep! sounds good!

you could use use tm.assert_produce_warning in this PR and add a comment to refer to the original 13188, so later could easily catch them! Please also do it for test_errorbar_plot test as well!

I believe that extension of tm.assert_produces_warning goes beyond the scope of the present PR.

yeah, a follow-up would be great!

As agreed with charlesdong1991, use assert_produces_warning and leave a comment on which warning message is emitted

Extract test_errorbar_plot_different_kinds from test_errorbar_plot and parametrize it.

ivanovmg · 2020-10-09T07:44:20Z

Did that. But also I decided to move from test_errorbar_plot part related to various kinds in a separate parametrize test. Can revert it if this is not suitable. But to me it seems that in general it would be required to split some test functions into smaller ones with more relevant names.

charlesdong1991 · 2020-10-09T07:52:51Z

I decided to move from test_errorbar_plot part related to various kinds in a separate parametrize test

sure!

charlesdong1991

very nice!

could you double check that after this change, the warnings you post in the PR description are gone?

cc @jreback

charlesdong1991 · 2020-10-09T08:17:08Z

pandas/tests/plotting/test_frame.py

+            _check_plot_works(tdf.plot, kind=kind, yerr=tdf_err, subplots=True)
+            fig = plt.gcf()
+            axes = fig.get_axes()
+            for ax in axes:


sorry, just noticed this, a minor question:

i think with tm.assert_produce_warning, we could still use the current way to check errorbars, right? _check_has_errorbars i recall can take in axes/list-like input.

axes = _check_plot_works( tdf.plot, kind=kind, yerr=tdf_err, subplots=True ) self._check_has_errorbars(axes, xerr=0, yerr=1)

Oh, yes, it does accept list-like. I will change to the original implementation.

ivanovmg · 2020-10-09T08:19:23Z

I checked.
Here is the output.

(base) root@f31dd4516194:/workspaces/pandas# pytest pandas/tests/plotting/test_frame.py
======================================================================================================================== test session starts =========================================================================================================================
platform linux -- Python 3.7.7, pytest-6.0.1, py-1.9.0, pluggy-0.13.1
rootdir: /workspaces/pandas, configfile: setup.cfg
plugins: cov-2.10.0, forked-1.2.0, xdist-1.34.0, hypothesis-5.23.11, asyncio-0.12.0
collected 191 items                                                                                                                                                                                                                                                  

pandas/tests/plotting/test_frame.py ...........................x....................................................................................................x..............................................................                            [100%]

========================================================================================================================== warnings summary ==========================================================================================================================
pandas/tests/plotting/test_frame.py::TestDataFramePlots::test_mpl2_color_cycle_str
  /workspaces/pandas/pandas/plotting/_matplotlib/style.py:64: MatplotlibDeprecationWarning: Support for uppercase single-letter colors is deprecated since Matplotlib 3.1 and will be removed in 3.3; please use lowercase instead.
    [conv.to_rgba(c) for c in colors]

-- Docs: https://docs.pytest.org/en/stable/warnings.html
======================================================================================================== 189 passed, 2 xfailed, 1 warning in 64.14s (0:01:04) ========================================================================================================
(base) root@f31dd4516194:/workspaces/pandas#

Another warning is handled in #36981

jreback · 2020-10-10T17:33:52Z

thanks @ivanovmg

FIX: fix cleanup warnings for errorbar timeseries

1c04f3c

jreback added Visualization plotting Clean Testing pandas testing functions or related to the test suite labels Oct 8, 2020

ivanovmg requested a review from charlesdong1991 October 9, 2020 03:15

charlesdong1991 reviewed Oct 9, 2020

View reviewed changes

pandas/tests/plotting/test_frame.py Outdated Show resolved Hide resolved

ivanovmg added 3 commits October 9, 2020 14:34

REF: refactor using assert_produces_warning

4083380

As agreed with charlesdong1991, use assert_produces_warning and leave a comment on which warning message is emitted

REF: catch warning in test_errorbar_plot

73bf910

REF: extract test_errorbar_plot_different_kinds

d5a9e5e

Extract test_errorbar_plot_different_kinds from test_errorbar_plot and parametrize it.

ivanovmg force-pushed the warnings/matplotlib-errorbar-timeseries branch from 8501cd3 to d5a9e5e Compare October 9, 2020 07:41

charlesdong1991 approved these changes Oct 9, 2020

View reviewed changes

charlesdong1991 reviewed Oct 9, 2020

View reviewed changes

ivanovmg added 2 commits October 9, 2020 15:23

REF: check errorbars for list-like axes

198fd1e

FIX: fix comments

762b9b9

ivanovmg mentioned this pull request Oct 9, 2020

BUG: fix matplotlib warning on CN color #36981

Merged

5 tasks

charlesdong1991 added this to the 1.2 milestone Oct 9, 2020

jreback merged commit 9425d7c into pandas-dev:master Oct 10, 2020

ivanovmg deleted the warnings/matplotlib-errorbar-timeseries branch October 10, 2020 18:11

This was referenced Oct 19, 2020

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

Closed

ENH: match warning messages #37261

Closed

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

FIX: fix cleanup warnings for errorbar timeseries (pandas-dev#36982)

3495372

ivanovmg mentioned this pull request Nov 6, 2020

TST: match matplotlib warning message #37666

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: fix cleanup warnings for errorbar timeseries #36982

FIX: fix cleanup warnings for errorbar timeseries #36982

ivanovmg commented Oct 8, 2020

jreback commented Oct 8, 2020

jreback commented Oct 8, 2020

ivanovmg commented Oct 9, 2020

charlesdong1991 commented Oct 9, 2020 •

edited

Loading

ivanovmg commented Oct 9, 2020

charlesdong1991 commented Oct 9, 2020

ivanovmg commented Oct 9, 2020 •

edited

Loading

charlesdong1991 commented Oct 9, 2020

ivanovmg commented Oct 9, 2020

charlesdong1991 commented Oct 9, 2020

charlesdong1991 left a comment

charlesdong1991 Oct 9, 2020

ivanovmg Oct 9, 2020

ivanovmg commented Oct 9, 2020

jreback commented Oct 10, 2020

FIX: fix cleanup warnings for errorbar timeseries #36982

FIX: fix cleanup warnings for errorbar timeseries #36982

Conversation

ivanovmg commented Oct 8, 2020

jreback commented Oct 8, 2020

jreback commented Oct 8, 2020

ivanovmg commented Oct 9, 2020

charlesdong1991 commented Oct 9, 2020 • edited Loading

ivanovmg commented Oct 9, 2020

charlesdong1991 commented Oct 9, 2020

ivanovmg commented Oct 9, 2020 • edited Loading

charlesdong1991 commented Oct 9, 2020

ivanovmg commented Oct 9, 2020

charlesdong1991 commented Oct 9, 2020

charlesdong1991 left a comment

Choose a reason for hiding this comment

charlesdong1991 Oct 9, 2020

Choose a reason for hiding this comment

ivanovmg Oct 9, 2020

Choose a reason for hiding this comment

ivanovmg commented Oct 9, 2020

jreback commented Oct 10, 2020

charlesdong1991 commented Oct 9, 2020 •

edited

Loading

ivanovmg commented Oct 9, 2020 •

edited

Loading