ENH: passing colors (and other optional keyword arguments) to violinplot() #3875
Conversation
56b26dd to
34bc624
Compare
34bc624 to
72b96d0
Compare
| @@ -6870,14 +6870,16 @@ def matshow(self, Z, **kwargs): | |||
|
|
|||
| def violinplot(self, dataset, positions=None, vert=True, widths=0.5, | |||
| showmeans=False, showextrema=True, showmedians=False, | |||
| points=100, bw_method=None): | |||
| points=100, bw_method=None, color='y', line_kw={}, | |||
There was a problem hiding this comment.
8000no mutable defaults, please. Just set it to None for default and then do an if-check early in the function to replace it with an empty dictionary.
|
Sorry about that, deleted my remote branch by mistake |
| @@ -7113,8 +7152,9 @@ def violin(self, vpstats, positions=None, vert=True, widths=0.5, | |||
| bodies += [fill(stats['coords'], | |||
| -vals + pos, | |||
| vals + pos, | |||
| facecolor='y', | |||
There was a problem hiding this comment.
(rhetorical question) how did we miss this in our original code review?!
|
I definitely agree with this in principle. We probably need to quibble over some of the specifics, but this does address a problem with the current design. |
| """Make a violin plot. | ||
|
|
||
| Call signature:: | ||
|
|
||
| violinplot(dataset, positions=None, vert=True, widths=0.5, | ||
| showmeans=False, showextrema=True, showmedians=False, | ||
| points=100, bw_method=None): | ||
| points=100, bw_method=None, color='y', line_kw=None, |
There was a problem hiding this comment.
should probably change this to None while touching this code.
There was a problem hiding this comment.
definitely would want to change color to None. Who knows? Maybe it might make sense to have color-cycling on this?
There was a problem hiding this comment.
Should I also drop the old default values for the line colors and fill alpha?
There was a problem hiding this comment.
That's a tough one to decide. There are no corollary values in the rcparams to pull from instead of these hard-coded values. But I hate hard-coded values so much!
There was a problem hiding this comment.
@WeatherGod I share your hatred for hard-coded values, but I also think all plots ought to look reasonably good with just the default parameters. If I get rid of the hard-coded values for facecolor, linecolor and alpha, the violins look a little bit lacklustre:
Do you reckon that's acceptable, or should I go back to the default values?
|
There are too many images commited, the |
93e3fa4 to
9a2f747
Compare
|
Can I ask if these changes will allow me to set the alpha for the violin body fill color independently of the body edge color? In matplotlib v1.4.0, the violin body is a |
|
It still seems violin plots don't allow diff colours? Does anyone want to pick this one up and champion? This looks 85% of the way there, so marking as good first issue if someone wants to take it on and incoprorate the comments above and rebase... |
|
Closing as abandoned, but does seem potentially useful, so hopefully someone takes it up. |
|
@jklymak Just as a remark: Nobody will find "good first issue" in closed PR. The default search will only go for open issues. If we want to advertise this as a "good first issue", we need to make an open issue that references this PR. |
I'm really glad that matplotlib now has a
violinplot()function, but I find the current implementation a bit annoying to customize, since the only way to change the colors, alpha values etc. is to modify each artist after creating the plot. One would ideally want to be able to pass in any arbitrary keyword arguments for thePolyCollectionandLineCollectionartists in each individual 'violin', but there's a trade-off between flexibility and simplicity.I think a reasonable compromise is to have the option to specify individual fill colors for each violin (like the
colorkwarg forhist()), but to have only a single dict for all the other optional kwargs for the fills and lines. For the time being I've stuck to the default colors/alpha values in the original implementation, but it might make more sense to leave these defaults unspecified so that they can be set according torcParams.