Warn if MPLBACKEND is invalid. #6699

Merged
merged 1 commit into from Jul 16, 2016

Conversation

Projects
None yet
5 participants
Contributor

anntzer commented Jul 6, 2016

I could be convinced to make this an error actually.

mdboom added the needs_review label Jul 6, 2016

Contributor

NelleV commented Jul 6, 2016

Hi Anthony,
Do you mind having the warning list the possible values? Something like:

("%s is not a valid backend. Valid backends are %s." % (s, the list of backends)

Else it looks good. I'd also be fine raising an error.

Contributor

anntzer commented Jul 6, 2016

Actually the error message already contains the list of valid backends (you can see it by calling explicitly matplotlib.use("foo")); I'm just turning it into a warning.
Let's have a straw poll as to whether this should be an error or a warning?

Contributor

NelleV commented Jul 6, 2016

I finally understand what is going on.

I think a warning is fine for now: that's the current behaviour when the matplotlibrc ba kend is set to an unknown backend so it is at least consistent with the behaviour when using matplotlibrc.

In general, I think it might be worth be more strict and throw errors when options are set to wrong values, but this behaviour needs to be consistent.

tacaswell added this to the 2.1 (next point release) milestone Jul 6, 2016

Owner

tacaswell commented Jul 7, 2016

I lean toward error as well.

Contributor

dopplershift commented Jul 7, 2016

"In the face of ambiguity, resist the temptation to guess." I'd say the zen argues for an error here.

Owner

tacaswell commented Jul 12, 2016

Consensus seems to be to raise if invalid.

@anntzer anntzer Throw an exception if MPLBACKEND is invalid.
6125d81
Contributor

anntzer commented Jul 12, 2016

That makes the patch even more trivial.

Contributor

NelleV commented Jul 12, 2016

Do you mind changing l. 1455 and l. 1105 (that will require more work) as well to have the error raised consistently.

Contributor

anntzer commented Jul 12, 2016

Changing l.1455 is probably a no-go (because other applications may be using matplotlib and using -dfoo themselves -- that's the whole reason -d$backend is deprecated I think).

Changing l.1105 comes down to switching the default value of fail_on_error to True, right?

Owner

tacaswell commented Jul 16, 2016

I agree on 1455 having to stay as it is for being able to pass things through.

I ma not sure about L1105, that makes all rcparsing raise. That should probably get wider discussion.

@tacaswell tacaswell merged commit c718e61 into matplotlib:master Jul 16, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 70.333%
Details

tacaswell removed the needs_review label Jul 16, 2016

anntzer deleted the anntzer:warn-on-invalid-MPLBACKEND branch Jul 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment