Fix validation of linestyle in rcparams and cycler. #15827
Conversation
2b5b24e to
664ab35
Compare
lib/matplotlib/rcsetup.py
Outdated
| except ValueError: | ||
| pass | ||
| try: | ||
| ls = ast.literal_eval(ls) |
There was a problem hiding this comment.
Is this only there to support reading rc_files? If so, I would mention this in a comment.
There was a problem hiding this comment.
That's literally the case for all functions in rcsetup.py...
There was a problem hiding this comment.
But most of them work implicitly because the don't parse nested structures. Then str -> other type is trivial.
Actually, I'd be in favor of making rcfiles YAML compliant. They mostly are already. We could delegate all the string parsing to the YAML parser. Then, the validators would only operate on the actual python types (not on their string representation). This would also be beneficial for using validators when setting rcParams directly in the code. That's for another time/discusion, but the comment would help to remove the string parsing from the validator later.
There was a problem hiding this comment.
Added a comment.
I am strongly opposed to using yaml here; it's the wrong tool for the task: it's not expressive enough to describe things that are actually problematic in the current rc syntax (axes.prop_cycler, path.effects), while bringing in a lot of complexity (a typical one being the many, many different kind of strings that exist) that is unwarranted.
There was a problem hiding this comment.
axes.prop_cycle could well be described by a dict
axes.prop_cycle:
color: ['1f77b4', 'ff7f0e', '2ca02c']
linestyle: ['-', '--', ':']
But no need to go into that discussion here.
Correcly use _validate_linestyle instead of validate_stringlist in the cycler linestyle validator. For compat, support both the (on, off, on, off, ...) and (offset, (on, off, on, off, ...)) forms. Don't support passing on, off, etc. as *individual* separate strings anymore as that's not a realistic use case (unlike passing the whole thing as a single string, which is supported for parsing the rc file).
664ab35 to
4899580
Compare
|
Anybody can merge after CI pass. |
Correcly use _validate_linestyle instead of validate_stringlist in the
cycler linestyle validator. For compat, support both the (on, off, on,
off, ...) and (offset, (on, off, on, off, ...)) forms. Don't support
passing on, off, etc. as individual separate strings anymore as that's
not a realistic use case (unlike passing the whole thing as a single
string, which is supported for parsing the rc file).
Closes #15825 (the corresponding test is that (0, [1, 2]) no longer fails but is instead parsed as (0, [1, 2])).
Makes https://github.com/matplotlib/matplotlib/pull/8040/files#diff-2423339af6767c3b30e1882f7c8d8f54R30 actually work (it didn't before).
Note that _validate_stringlist needed to be moved up in rcsetup.py to come before the _prop_validators dict.
Edit: Also closes #9792 and supersedes #9797, from which some test cases were taken.
PR Summary
PR Checklist