Added bw_method to sm.graphics.violinplot + default colors?

[olgabot]

Hello there,
I have a homebrewed version of this (also based off of the same code!) but it allows the user to specify the bandwidth method. So in this PR, I added the ability to change the bandwidth method. It's currently with the regular args because I didn't think this is a plot argument, but rather an estimation argument. Suggestions?

Here are some examples: http://nbviewer.ipython.org/gist/olgabot/7915540

Besides the bandwidth method, can the default facecolor also be changed? The yellow is rather ugly, and I'd much prefer something from colorbrewer: http://bl.ocks.org/mbostock/5577023

Thanks,
Olga

PS Initially I was going to add this to pandas (pandas-dev/pandas#5676) but since statsmodels already has most of the code, this seemed like a more natural choice.

[coveralls]

Coverage remained the same when pulling 4a52425 on olgabot:violinplot_bandwidth into a14d989 on statsmodels:master.

[josef-pkt]

Thanks Olga,

The change looks good, I also think it should be a regular keyword argument, as you added it.
Can you add a test case? Those are currently just smoke tests to check that an option doesn't raise an exception.

I also agree about the color, I like your bluish much better. But maybe someone else also has an opinion, I'm not an "expert" on default color choices.

[josef-pkt]

Hi Olga,
If you want you can also add the change of the default color to this PR, or make a new one.
I asked on the mailing list and Ralf, the author of the violinplot functions also likes your color better.

[olgabot]

Great!

I don't claim to be a color expert either, so I stick to the colors that experts have picked out such as colorbrewer: http://bl.ocks.org/mbostock/5577023

There's a colorbrewer extension for Python ([brewer2mpl](https://github.com/jiffyclub/brewer2mpl/wiki)) which I use in my plotting library, [prettyplotlib](https://github.com/olgabot/prettyplotlib). Matplotlib also has the colormaps built in, but it's annoying to access individual colors, rather than as a cmap.

I also have a question about graphics in general. Many of the plots are ggplot2-like, is that because its' what people like in R?

[josef-pkt]

I would call our "style" so far eclectic, and not "ggplot2-like"

The plots in statsmodels were written by different authors, mostly based on whatever they found as reference.
Our emphasis has been on statistics plot, which brings us into the area of ggplot2-like or Stata like or SAS like.

We try to have a common pattern for the interface, but nobody has reviewed the statsmodels "style" yet. There are other packages, your prettyplotlib looks like that, that emphasis style and convenience over statistics, while so far our emphasis is reversed

We use the cmaps in some of the plots.

[jseabold]

Sorry for the slow review here. Can you add something to the documentation about bw_method. Copying mostly from scipy is fine.

bw_method : str, scalar or callable, optional
    The method used to calculate the estimator bandwidth.  This can be
    'scott', 'silverman', a scalar constant or a callable.  If a scalar,
    this will be used directly as `kde.factor`.  If a callable, it should
    take a `gaussian_kde` instance as only parameter and return a scalar.
    If None (default), 'scott' is used.  See scipy.stats.gaussian_kde for more 
    information.

[jseabold]

I think bw_method should go after plot_opts for backwards compatibility

[jseabold]

Closing this. Please re-open if you find the time to address the comments.

[josef-pkt]

I started to work on this, but it needs more KDE option.

[jseabold]

Why does your work need this PR open?

[josef-pkt]

Because if there is no open issue or PR, then I forget about it.
(open issues and PRs are the TODO list for the next 5 years.)

[jseabold]

Then let's keep it in an issue. I'm trying to clean up the PRs.

[josef-pkt]

Closing PRs that are not obsolete and superseded by something else is not useful.

I like a todo list and not a clean issue tracker.