Well Travis is really angry at me for theb'dotted' test case ^^! (Too bad, the tests pass locally on my computer with Python 2.7…) I am going to investigate this.

Ok, it looks like Python 2 (, which I am working with) is less strict about string-like object comparisons:

# Python 2.7.12 |Continuum Analytics, Inc.| (default, Jul  2 2016, 17:42:40)In [1]:b'dotted'in [u'dotted']Out[1]:TrueIn [2]:'dotted'.encode('ascii')in [u'dotted']Out[2]:True

than Python 3:

# Python 3.6.0 |Continuum Analytics, Inc.| (default, Dec 23 2016, 12:22:00)In [1]:b'dotted'in ['dotted']Out[1]:FalseIn [2]:b'dotted'.decode()in ['dotted']Out[2]:TrueIn [3]:'dotted'.encode('ascii').decode()in ['dotted']Out[3]:True

The commit973dd5e should fix the previous failures, by using thedecode method if it is available.

Hum, Travis seems a bit happier with Python 3, but I just tried locally_validate_linestyle('dotted'.encode('utf-16') on Python 3, and now I understand better@tacaswell's comment in#8145 😞… It raises a niceUnicodeDecodeError as the enconding is not UTF-8.

What is the usual way to deal with this kind of encoding problem?

PS: I'll take take of the PEP8 issue, do not worry.

Ok, I had a look at howmatplotlib.text.Text handles the problem of encoding. It accepts “string or anything printable with '%s' conversion”. So actually, I wonder if it would not be better to do something similar in_validate_linestyle if we want to avoid avoid raising aUnicodeDecodeError exception when_validate_linestyle is given a “string” with an “exotic” encoding. For example

try:return_validate_named_linestyle("{:s}".format(ls))exceptKeyError:# ls is not a valid name of line style.pass

should avoid raising aUnicodeDecodeError with UTF-16 & Co. Or would it be better to really raise aUnicodeDecodeErrorexception? In this case,

try:return_validate_named_linestyle(six.text_type(ls))exceptKeyError:# ls is not a valid name of line style.pass

may be more consistent with the other string checking done inrcsetup rather than trying to call thedecode method if it is available (like in973dd5e).

Removing theneed_review flag for the moment because handling encoding has appear to be a bit harder than I expected and the proper way to do it has still to be chosen.

Just pass the user input through, in 3, users passing bytes in is wrong and should fail, on 2 it happens to work due to the warts of string v unicode on python2.

Ok thanks for the comment@tacaswell . I'll do this in a moment.

Note to myself: go back to before the decode commit, and play withsix.PY3 in the test.

Rebased! I squashed some similar commits and dropped the former one that attempted to decode the arguments.

@tacaswell No more attempt to decode the arguments, and I modified the tests to take care of the difference between Python 2 and Python 3. TBH, I struggled a bit to find a clean way to do the latter: I hope the style of the new version of the tests is fine. On my local computer with Python 2, the tests are passing (and the doc is building): hopefully Travis with Python 3 will agree.

The new approach may not play nicely (as in, cause a warning) due to the same issue as#7658? I haven't checked, but you may want to have a look.

@anntzer Thanks, I'll have a look this weekend.

Based on@anntzer's remark, it appears that indeed

np.array([1,2])in ['dotted','solid']

raises aFutureWarningbecause one day the comparison that is done under the hood byin will be element-wise (the warning will then become aValueError exception, which does not belong to the ones that are currently captured in this PR). Actually, in the precise case of this PR, thisFutureWarning is not raised because we ignore the case, which calls.lower() before anyin operation, and thus raises anAttributeError error that is correctly caught if a Numpy array is given as on-off sequence.

With97234b5, I reintroduce some early type checking to avoid any comparison of a Numpy array with strings. I agree it is kind of a nuke for a problem that does not even currently exist, but some (super) thin reasons to support this may be:

• it makes the exception catching clearer (one can remove theAttributeError that may be raised by.lower())
• if one day one wantnot to ignore the case of the argument, it will still work properly… (eventhough this API break is not very likely to happen ;) )
• well, it works as well as the other solution (at least on Python 2 the tests pass locally, I hope Travis will agree on Python 3 ^^).

@tacaswell Ping if you want to have a look when Travis will have run (TL; DR:decode is no more 🐑)

@anntzer I hope you will notice the effort ofnot usingis_string_like, despite the urge of doing so on first thought 😁. (ref:#8011)

@afvincent Congratulations, you've earned un bon point. (Dix points = une image. Dropping Python2 support = many many points :-))

@anntzer You know, I had a few teachers that were doing so in primary school 😄.

Abouy dropping Python2, well… Let's say that I made a “mistake” when I started to learn Python at the beginning of my Ph.D. But I promise, I'll switch for my next big research project (2020 is coming close anyway…). We'll see if the bounty points lasts until then ;). If it doesn't, at least I'll have access to all the wonderful things like @, f-strings & Co!

You know, I grew up in France... (in fact, pretty close to where you did your PhD...)

Rahh, Travis does not seem to be happy (@anntzer you may want to take yourbon point back). I think I understand why (if I am right, I was lucky about exceptions catching before…): I'll push a commit when I will have a fix.

Rebased (and squashed) with a new version! The former spirit is still there but now the exception catching should be more robust, both on Python 2 (a local pytest runs smoothly withtest_rcparams.py and the docs are building) and on Python 3 (manual testing in an interactive session, I hope Travis will agree with me this time).

For the record, previously on both Python versions, the exception handling was flaky or error-prone. For example on Python 2, aUnicodeDecodeError was not caught with arguments like'dotted'.encode('utf-16'), causing a short exit that was not expected. Unfortunately, this exception was then caught by pytest, which was expecting aValueError and was thus marking the test as fine… On Python 3, there was (at least) a funny doubly wrong behavior: byte-likels argument that were of even-length were passed to the instance ofvalidate_nseq_float, which was converting them into funny on-off ink sequences (for examplevalidate_nseq_float()('dotted'.encode('utf-16')) corresponds to[255.0, 254.0, 100.0, 0.0, 111.0, 0.0, 116.0, 0.0, 116.0, 0.0, 101.0, 0.0, 100.0, 0.0] 😄 )

Ok, now Travis stays calm :)! Ping to@tacaswell if you want to have a look, now that Travis is peacefull. And@anntzer because there have been quite a few changes since you had a look yesterday (sorry for that 🐑 ).

Re-rebased: squashed to 2 single commits and reordered them (1. code changes, 2. update of the related test).

The only difference with the previous commits should be:

• hasattr(ls, 'decode') <-isinstance(ls, (bytes, bytearray)), because “Explicit is better than implicit;”
• more useful exception messages for debugging by adding the “representation” of thels argument (done with"Yada yada {!r} yada".format(ls), because 2020 is far away and I likeformat :) ).

This looks good to me.

Thanks!

Thanks to all who reviewed the PR :). It was a windy journey, but an instructive one from my side ^^.

You have hit on both why the bytes / unicode changes were made in python3 and why the transition has been a bit rough 😈 .

There does not appear to be a_validate_linestyle inv2.0.x;@afvincent can you verify if this needs backporting?

@QuLogic Indeed, backporting does not seem required.#8040 (which is fixed by this PR) was not backported but milestoned for 2.1 (by@tacaswell) instead: that is why there is no_validate_linestyle in v2.0.x branch ⇒ removing theneed_backport flag.