8000 ENH: passing colors (and other optional keyword arguments) to violinplot() by alimuldal · Pull Request #3875 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

ENH: passing colors (and other optional keyword arguments) to violinplot() #3875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

ENH: passing colors (and other optional keyword arguments) to violinplot() #3875

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72b96d0
pass different colors (+ additional line/polycollection kwargs) to vi…
alimuldal Dec 3, 2014
8cd2225
replace dict defaults with None
alimuldal Dec 3, 2014
9a2f747
added unittests (w/o test images)
alimuldal Dec 3, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
replace dict defaults with None
  • Loading branch information
@alimuldal
alimuldal committed Dec 3, 2014
commit 8cd22253cd623a2ac5139e937369afd11a514586
11 changes: 7 additions & 4 deletions lib/matplotlib/axes/_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6870,15 +6870,15 @@ 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, color='y', line_kw={},
points=100, bw_method=None, color='y', line_kw=None,
**fill_kw):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fill_kw -> kwargs if you want to leave this as-is, but I don't really see why to treat the line properties differently from the fill properties.

"""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, color='y', line_kw={},
points=100, bw_method=None, color='y', line_kw=None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably change this to None while touching this code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely would want to change color to None. Who knows? Maybe it might make sense to have color-cycling on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I also drop the old default values for the line colors and fill alpha?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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:

default_violins

Do you reckon that's acceptable, or should I go back to the default values?

**fill_kw):

Make a violin plot for each column of *dataset* or each vector in
Expand Down Expand Up @@ -6986,14 +6986,14 @@ def _kde_method(X, coords):

def violin(self, vpstats, positions=None, vert=True, widths=0.5,
showmeans=False, showextrema=True, showmedians=False,
color='y', line_kw={}, **fill_kw):
color='y', line_kw=None, **fill_kw):
"""Drawing function for violin plots.

Call signature::

violin(vpstats, positions=None, vert=True, widths=0.5,
showmeans=False, showextrema=True, showmedians=False,
color='y', line_kw={}, **fill_kw):
color='y', line_kw=None, **fill_kw):

Draw a violin plot for each column of `vpstats`. Each filled area
extends to represent the entire data range, with optional lines at the
Expand Down Expand Up @@ -7124,6 +7124,9 @@ def violin(self, vpstats, positions=None, vert=True, widths=0.5,
elif len(color) != N:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure we want to check the length, this will prevent passing in things like intertools.cycle(['r', 'g', 'b']).

I also think that there are tools in cookbook that do this with all of the bells and whistles/corner cases covered.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the reason for separating them, and there is some precedence for it. This plotting function produces a mix of artist types, each of which may need to be stylized differently. If I pass a linewidth argument, am I referring to the width of the lines for the polygon edges or what?
We do this for plt.subplots(), and there isn't even any ambiguity there. We allow for axes kwargs to be its own dictionary, and then remaining kwargs go to the figure constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WeatherGod Indeed, and I think that would actually be quite a common use case. For example, it's unlikely that I'd want to set the same line color for the polygon edges and the lines marking the medians etc. since they would have poor color contrast. In fact I was specifically thinking of plt.subplots() when I was trying to decide how best to pass kwargs to heterogeneous output types.

raise ValueError(datashape_message.format("color"))

if line_kw is None:
line_kw = {}

# original default values for line color and alpha
line_color = line_kw.pop('colors', 'r')
alpha = fill_kw.pop('alpha', 0.3)
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/pyplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3420,7 +3420,7 @@ def triplot(*args, **kwargs):
@_autogen_docstring(Axes.violinplot)
def violinplot(dataset, positions=None, vert=True, widths=0.5, showmeans=False,
showextrema=True, showmedians=False, points=100, bw_method=None,
color='y', line_kw={}, hold=None, **fill_kw):
color='y', line_kw=None, hold=None, **fill_kw):
ax = gca()
# allow callers to override the hold state by passing hold=True|False
washold = ax.ishold()
Expand Down
0