#10282 deprecated rcparamsbackend.qt4 andbackend.qt5 but left the defaults active so the deprecation warning came on, even if the user did not have these rcparams in thematplotlibrc. This PR makes the defaultsNone, and adds a check forNone in the validator (that returns the old defaults). Hence the deprecation now only displays if these rcParams are still in thematplotlibrc.

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

jklymak requested review fromanntzer andefiring

January 30, 2018 18:50

Copy link

MemberAuthor

jklymak commentedJan 30, 2018

Note thattutorials/introductory/usage.py is also now wrong.

matplotlib/tutorials/introductory/usage.py

Line 495 in482bde0

# How do I select PyQt4 or PySide?

jklymak added this to thev2.2 milestone

Jan 30, 2018

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

Jan 30, 2018

Copy link

MemberAuthor

jklymak commentedJan 30, 2018

Milestoning as release critical if#10282 is going to stay in...

anntzer approved these changes

Jan 30, 2018

View reviewed changes

Copy link

Contributor

anntzer left a comment

Choose a reason for hiding this comment

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

I think that's a much better way to do it indeed.

Copy link

Member

efiring commentedJan 30, 2018

Failures:

/home/travis/build/matplotlib/matplotlib/lib/matplotlib/rcsetup.py:274:10: E711 comparison to None should be 'if cond is None:'    if s == None:         ^/home/travis/build/matplotlib/matplotlib/lib/matplotlib/rcsetup.py:287:10: E711 comparison to None should be 'if cond is None:'    if s == None:         ^

FIX: deprecate qt4/5 rcparams

17112e8

jklymak force-pushed thefix-deprecate-qt45rcparams branch from047aa18 to17112e8Compare

January 30, 2018 21:10

Copy link

MemberAuthor

jklymak commentedJan 30, 2018

Sorry - shouldn't ask for reviews before I make sure the tests pass.

Copy link

MemberAuthor

jklymak commentedJan 30, 2018

@anntzer does this mean we can remove the somewhat complicated testing check? I left it in for now...

Copy link

Contributor

anntzer commentedJan 30, 2018

Probably the warnings suppression inmatplotlib/__init__ too.

FIX: deprecate qt4/5 rcparams

3cce6f8

Copy link

MemberAuthor

jklymak commentedJan 30, 2018

I didn't see what needs changing in__init__.py...

Copy link

Contributor

anntzer commentedJan 30, 2018

https://github.com/matplotlib/matplotlib/pull/10282/files#diff-9ffb0d1db51a67ee4f37528e00715b3cR1119 andhttps://github.com/matplotlib/matplotlib/pull/10282/files#diff-9ffb0d1db51a67ee4f37528e00715b3cR1160

Copy link

MemberAuthor

jklymak commentedJan 30, 2018

I guess I wasn't saying we had to get rid of all the warning suppression. I don't quite understand what those two sections do. Feel free to push onto this PR, or open one and close thsi one.

Copy link

Contributor

anntzer commentedJan 30, 2018

Leaving them in is a bit redundant but not a critical problem I think.

efiring approved these changes

Jan 31, 2018

View reviewed changes

efiring merged commit5a58d8d intomatplotlib:master

Jan 31, 2018

Copy link

Member

QuLogic commentedFeb 4, 2018

Isn't this still happening onthe Python 3.6 build? Sure is annoying when running pytest.

Copy link

MemberAuthor

jklymak commentedFeb 4, 2018

I’m a little confused about pytest. Does it look at the local matplotlibrc? If so, if you still have that rcParams in your matplotlibrc you’ll get the warnings.

Copy link

Member

QuLogic commentedFeb 4, 2018

Test setup callsrcdefaults, which re-applies all rcParams. I guess it takes the final result somehow instead ofNone.

Copy link

MemberAuthor

jklymak commentedFeb 4, 2018

Ok well we should put the test logic back then I guess. I do t quite get why this is happening though.

Copy link

Contributor

anntzer commentedFeb 4, 2018

Won't have much time to take care of this (or other prs) in the coming days. Will get back later, but if this is blocking a release feel free to revert the original deprecation (sorry for the sloppy patch).

jklymak deleted the fix-deprecate-qt45rcparams branch

February 4, 2018 22:20

Copy link

MemberAuthor

jklymak commentedFeb 4, 2018

OK,backend.qt4 andbackend.qt5 need to be added to_obsolete_set in__init__.py I think, though I'm a little confused about the differences between_obsolete_set and_deprecated_map.