NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

FIX: catch warnings from pandas in cbook._check_1d#16347

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

jklymak merged 3 commits intomatplotlib:masterfromtacaswell:fix_pandas_index_deprecation

Jan 28, 2020

Merged

FIX: catch warnings from pandas in cbook._check_1d#16347

jklymak merged 3 commits intomatplotlib:masterfromtacaswell:fix_pandas_index_deprecation

Jan 28, 2020

Conversation

Copy link

Member

tacaswell commentedJan 27, 2020•
edited
Loading

If we catch the warning or the exception, we need to cast to numpy
because later on in_plot_args we again use multi-dimensional
indexing to up-cast to a 2D array.

closes#16295

PR Summary

PR Checklist

Has Pytest style unit tests
Code isFlake 8 compliant

anntzer reviewed

Jan 27, 2020

View reviewed changes

lib/matplotlib/cbook/__init__.py

		with warnings.catch_warnings() as w:
		warnings.filterwarnings("ignore",
		category=DeprecationWarning,
		module='pandas[.*]')

Copy link

Contributor

anntzerJan 27, 2020

Choose a reason for hiding this comment

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

does this mean we're just kicking the can down the road and will need yet another fix when pandas actually change this behavior?

Copy link

MemberAuthor

tacaswellJan 27, 2020

Choose a reason for hiding this comment

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

The change will be to raise one of the Exceptions we already catch below so we will still be happy.

Copy link

Contributor

anntzerJan 27, 2020

Choose a reason for hiding this comment

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

fair enough

FIX: catch warnings from pandas in cbook._check_1d

c7eddd6

If we catch the warning or the exception, we need to cast to numpybecause later on in `_plot_args` we again use multi-dimensionalindexing to up-cast to a 2D array.closesmatplotlib#16295

tacaswell force-pushed thefix_pandas_index_deprecation branch fromae4354c toc7eddd6Compare

January 27, 2020 23:33

tacaswell added this to thev2.2.5 milestone

Jan 27, 2020

tacaswell added the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label

Jan 28, 2020

Copy link

MemberAuthor

tacaswell commentedJan 28, 2020

@bashtage However on older versions we are not getting back a numpy array

In [1]:importpandasaspdIn [2]:d=pd.Series([])In [3]:d.index[:,None]Out[3]:Int64Index([],dtype='int64')In [4]:type(d.index[:,None])Out[4]:pandas.core.indexes.numeric.Int64IndexIn [5]:d.index[:,None].ndimOut[5]:1In [6]:pd.__version__Out[6]:'0.25.1'

story645 requested changes

Jan 28, 2020

View reviewed changes

lib/matplotlib/cbook/__init__.py Outdated

Comment on lines 1372 to 1373

		# was an unintentional quirk of pandas the above line will raise
		# an exception in the future.

Copy link

Member

story645Jan 28, 2020

Choose a reason for hiding this comment

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

since the above line is getting deleted, this comment is no longer relevant.

Copy link

MemberAuthor

tacaswellJan 28, 2020

Choose a reason for hiding this comment

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

It is still relevant, the line above was moved down and put inside the context manager below.

Copy link

Member

story645Jan 28, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Ok, then it should be changed to "the line below" or moved to be next to the line it's referring to:

ndim=x[:,None].ndim#will raise exception in future

lib/matplotlib/cbook/__init__.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_axes.py OutdatedShow resolvedHide resolved

tacaswell added2 commits

January 28, 2020 15:39

DOC: clarified comments in code about what this code is doing

676f3eb

TST: renamed test to be less idiomatic

77e7b31

tacaswell requested a review fromstory645

January 28, 2020 21:07

story645 approved these changes

Jan 28, 2020

View reviewed changes

tacaswell mentioned this pull request

Jan 28, 2020

Multi-dimensional Index indexing followupspandas-dev/pandas#30867

Closed

jklymak approved these changes

Jan 28, 2020

View reviewed changes

jklymak merged commita7501f9 intomatplotlib:master

Jan 28, 2020

Copy link

lumberbot-appbot commentedJan 28, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

$ git checkout v2.2.x$ git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

$ git cherry-pick -m1 a7501f94988b952bd97a7b382b86c924cf464f62

You will likely have some merge/cherry-pick conflict here, fix them and commit:

$ git commit -am 'Backport PR #16347: FIX: catch warnings from pandas in cbook._check_1d'

Push to a named branch :

git push YOURFORK v2.2.x:auto-backport-of-pr-16347-on-v2.2.x

Create a PR against branch v2.2.x, I would have named this PR:

"Backport PR#16347 on branch v2.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free tosuggest an improvement.

lumberbot-appbot added the status: needs manual backport label

Jan 28, 2020

Copy link

lumberbot-appbot commentedJan 28, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

$ git checkout v3.1.x$ git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

$ git cherry-pick -m1 a7501f94988b952bd97a7b382b86c924cf464f62

You will likely have some merge/cherry-pick conflict here, fix them and commit:

$ git commit -am 'Backport PR #16347: FIX: catch warnings from pandas in cbook._check_1d'

Push to a named branch :

git push YOURFORK v3.1.x:auto-backport-of-pr-16347-on-v3.1.x

Create a PR against branch v3.1.x, I would have named this PR:

"Backport PR#16347 on branch v3.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free tosuggest an improvement.

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request

Jan 28, 2020

Backport PRmatplotlib#16347: FIX: catch warnings from pandas in cboo…

ff8b1a9

…k._check_1d

meeseeksmachine mentioned this pull request

Jan 28, 2020

Backport PR #16347 on branch v3.2.x (FIX: catch warnings from pandas in cbook._check_1d)#16360

Merged

tacaswell deleted the fix_pandas_index_deprecation branch

January 28, 2020 23:22

tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request

Jan 28, 2020

Backport PRmatplotlib#16347: FIX: catch warnings from pandas in cboo…

9859fa2

…k._check_1dMerge pull requestmatplotlib#16347 from tacaswell/fix_pandas_index_deprecationFIX: catch warnings from pandas in cbook._check_1d Conflicts:lib/matplotlib/tests/test_axes.py         - new test in middle of section of tests not on v2.2.x yet, only kept           new test

tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request

Jan 28, 2020

Backport PRmatplotlib#16347: FIX: catch warnings from pandas in cboo…

fa8ac63

…k._check_1dMerge pull requestmatplotlib#16347 from tacaswell/fix_pandas_index_deprecationFIX: catch warnings from pandas in cbook._check_1d

This was referencedJan 28, 2020

Backport PR #16347: FIX: catch warnings from pandas in cbook._check_1d#16361

Merged

Backport PR #16347: FIX: catch warnings from pandas in cbook._check_1d#16362

Merged

tacaswell added a commit that referenced this pull request

Jan 29, 2020

Merge pull request#16360from meeseeksmachine/auto-backport-of-pr-16…

89ff308

…347-on-v3.2.xBackport PR#16347 on branch v3.2.x (FIX: catch warnings from pandas in cbook._check_1d)

timhoffm added a commit that referenced this pull request

Jan 29, 2020

Merge pull request#16362from tacaswell/auto-backport-of-pr-16347-on…

bbf4c7c

…-v3.1.xBackport PR#16347: FIX: catch warnings from pandas in cbook._check_1d

timhoffm added a commit that referenced this pull request

Jan 29, 2020

Merge pull request#16361from tacaswell/auto-backport-of-pr-16347-on…

d0cdf97

…-v2.2.xBackport PR#16347: FIX: catch warnings from pandas in cbook._check_1d

Copy link

Member

jklymak commentedJan 30, 2020

So,test_bar_pandas is still failing, at least w/#16286 which I rebased, so it should include this fix?

Copy link

Member

jklymak commentedJan 30, 2020

I guess because pandas 1.0 is out?

Copy link

Member

jklymak commentedJan 30, 2020

I actually don't understand. The warning is from pandas 1.0 (old pandas pass this test) and the result is:

E           DeprecationWarning: Support for multi-dimensional indexing (e.g. `index[:, None]`) on an Index is deprecated and will be removed in a future version.  Convert to a numpy array before indexing instead.

So somehow we are no longer capturing the warning.