Fix #4855: Blacklist rcParams that aren't style #5440
Conversation
| def is_style_file(filename): | ||
| """Return True if the filename looks like a style file.""" | ||
| return STYLE_FILE_PATTERN.match(filename) is not None | ||
|
|
||
|
|
||
| def _apply_style(d): | ||
| mpl.rcParams.use(blacklist_style_params(d)) |
|
Should also remove all of the black-listed entries from |
|
@tacaswell: Good point about removing blacklisted entries from the classic style. Done. |
|
I think this one is good to go. |
| @@ -71,18 +96,18 @@ def use(style): | |||
|
|
|||
| for style in styles: | |||
| if not cbook.is_string_like(style): | |||
| mpl.rcParams.update(style) | |||
| _apply_style(style) | |||
| continue | |||
| elif style == 'default': | |||
| mpl.rcdefaults() | |||
There was a problem hiding this comment.
There is a bit of a disconnect here. rcdefaults() is more than just style stuff, but it doesn't make sense to blacklist entries there, either, does it?
There was a problem hiding this comment.
@mdboom, I think @WeatherGod has a good point here: 'default' style should modify only non-blacklisted things, shouldn't it?
|
I think this raises a bit more general question of what is the line between styles and settings. There are tons of other settings such as the default keymappings, animation settings, and such that may or may not make sense as a "style". I am fine with what has been chosen so far, it is just a question of how much more do we want to blacklist. |
|
Also, do we know if people have been using style.use() as a convenient way to apply custom rcParams? If so, then this change could break that use-case (not that it would have been a good thing to do in the first place, though) |
No, but I know of at least three cases where style was used to turn interactive on and caused end users no end of trouble... ;) |
|
This is all fine, but we still don't have a definition that can help guide us in choosing what should and shouldn't be allowed in a style file. So, to get the conversation rolling, I'll propose a definition: "a non-style parameter is one whose change would have no effect after importing pyplot". This definition might not be good enough. Should the line be drawn at figure creation? axes creation? |
|
My definition is things that affect the appearance of the plot are style. Things that are related to the environment/platform in use and other technical considerations are not. |
|
@tacaswell: Any thoughts on the definition of "style parameters" here? |
|
I would say it has to have a visual effect, rather than a ui effect (interactive, key bindings, default file types) which i think lines up with what @mdboom said and is a super set of @WeatherGod. Things like dpi are on the edge and should probably be allowed. |
|
I've added a comment about what should be blacklisted. I hope that addresses the comments above, and otherwise this is good-to-merge. |
|
I think this is ready-to-merge, unless there are other comments. |
|
@mdboom, see @WeatherGod's first inline comment. |
Fix #4855: Blacklist rcParams that aren't style
|
I have the impression that some might want tighter restrictions--a larger blacklist. Merging this does not preclude such changes, but it seems worthwhile to get the mechanism in place without further delay. |
|
Should this be backported to 2.0? |
|
yes. |
Fix matplotlib#4855: Blacklist rcParams that aren't style
Needs a what's new