ENH/REF: Overhauled boxplots by phobson · Pull Request #2643 · matplotlib/matplotlib

phobson · 2013-12-03T15:20:37Z

In a nut shell here is what has happened:

Created a function (cbook.boxplot_stats) that take any data that axes.boxplot could handle and generates a list of dicts containing the statistics needed to draw a set of boxplots.
Then I created a new method (axes.bxp) that accepts dictionaries from cbook.boxplot_stats and allows the user to customize how the plots are rendering. In other words, dictionaries of style **kwargs can be passed configuring the style of the boxes/whisker/caps, the outliers, the medians, the means (new functionality).
The new bxp method also allows the user to toggle the drawing of any element of the boxplot via **kwargs (i.e., shownotches, showboxes, showfliers, showmeans, showcaps).
The new bxp looks for a label key in the dictionaries. If it's available, it'll use that as the x-tick label. Otherwise, the labels default to the boxplot's position.
And finally, the original boxplot method is functionally unchanged, but now it relies on cbook.boxplot_stats and axes.bxp to do all the work.
Added a bunch of tests

Potential remaining items:

~~Well, I definitely need to draft up a good example to show all of this off. That's coming shortly (pending feedback on the existing portions of the PR).~~
We might want to expose the full capability of bxp in boxplot (i.e., the style and show* kwargs). Currently, boxplot's API/capabiltiy is unchanged. I'm fine with this but I'll need to pretty much duplicate all of the bxp tests to make sure nothings gets lost in translation, (done)
~~Rerunning boilerplate.py didn't seem to pick up on bxp. Do I need to clear it out first?~~
~~We might want to add boxplot_stats to pyplot and pylab.~~ (not gonna happen)
~~And it looks like boilerplate.py added a bunch of unicode strings to the call sigs and now the 3.2 build is failing (dang it).~~ (fixed that)

This PR addresses the following issue (or allows users to address them themselves):

There is all #1027 out there. While we don't address that issue directly. I personally agree with matplotlib's approach to boxplots that brings up that issue. In other words, that's a feature, not a bug, IMO

…tats) and a drawer (axes.bxp). Existing function now relies on these two functions.

TST: added tests for all of bxp and more boxplot tests TST: fixed boxplot baseline image TST: added baseline images for bxp

…o outliers as fliers

ENH/TST: boxplots stats now takes a labels kwarg MNT: minor PEP8 whitespace fix

tacaswell · 2013-12-03T16:06:51Z

I don't think it is worth re-fitting boxplot to replicate the new functionality. It is important to keep it's interface as-is for backwards compatibility, but point users who want the new features to the new function (which is where the examples come in ;) )

You need to add it to the list _plotcommands in boilerplate.py to get picked up.

There was discussion about doing away with pylab in the last hangout and I don't think it is a good fit in pyplot.

phobson · 2013-12-03T16:40:52Z

@tacaswell To be fair, I think we can add the new stuff to boxplot without breaking anything. We just have to add bxp's kwargs to boxplot's call signature, document them, and pass them directly to bxp (which already happens in a very limited fashion),

Thanks for the other tips. I'll look into them soon.

…) or specified percentiles. If IQR is zero, min/max are used

tacaswell · 2013-12-13T17:48:37Z

Would it be better to have this logic shuffle/flip the data and only have the plotting once rather than creating inner functions which do both the data munging and the plotting (sorry if it become obvious later why you did it this way).

@tacaswell I could definitely rearrange that as you describe. this is just an vestige of the original boxplot code. luckily I think i have enough tests now that I'll know if I break something :)

@tacaswell, my first reaction probably would be like yours, but looking farther down in the code, I think that using these local functions is actually the cleanest way to encapsulate the axis flip.

tacaswell · 2013-12-13T17:55:32Z

I left a few picky comments that other should comment on before you do anything about.

I am also not sure about the * in the doc strings.

phobson · 2013-12-13T17:59:07Z

@tacaswell thanks for the feedback. As you suggested, I'll wait for more comments. The *s in the docstring are also vestiges of the original code. I think it'll be safe to proceed on converting the docstrings to the numpy format.

tacaswell · 2013-12-13T20:09:17Z

Your comment re pandas does raise the interesting point that matplotlib.financial will currently make candlestick plots which should maybe be re-written using this. On the other hand, that module will be pulled out in 1.5.

This looks like a really nice PR.

phobson · 2013-12-13T20:14:55Z

Happy to take a look at the candlestick, as it should be pretty easy. However, that'll have to happen in a separate PR if the Repo Collabs decide it's worth it. Can you provide some guidance with my examples? Namely, do I need to include the output figures or does the doc-building process take care of that?

tacaswell · 2013-12-13T21:30:50Z

pretty sure the doc-build process takes care of that, but I have not done much with it. The easy thing is to run the doc-build locally and see if it takes care of it.

Yes, I was not suggesting you do the candlestick in this PR (or even that you should do it). It would be good to find a person who does finance to take care of that module/toolkit, but that is irrelevant for this PR.

…ild with docs

phobson · 2014-01-02T19:41:05Z

@tacaswell
Just chiming in to say that I built the docs and the new examples were included. Any other feedback, suggestions, requests, etc are welcome!

tacaswell · 2014-01-02T21:48:15Z

@phobson This needs a re-base.

It looks good to merge to me, but I don't want to push the button on this much new code without someone else looking at it.

phobson · 2014-01-02T23:17:19Z

@tacaswell do i need to rebase to squash the commits, to sync with current master, or both?

efiring · 2014-01-18T23:00:44Z

Maybe "each of the artists"? This is still not very clear.

ENH/REF: Overhauled boxplots

tacaswell · 2014-01-24T06:32:14Z

@phobson Merged! Thanks for this.

phobson · 2014-01-24T06:33:18Z

@tacaswell wooo! thanks for the guidance and patience

phobson added 10 commits December 2, 2013 10:39

REF/TST: split up boxplot function into a calculator (cbook.boxplot_s…

8bf8653

…tats) and a drawer (axes.bxp). Existing function now relies on these two functions.

BUG: fixed cbook tests for Python 2.6

b429dcf

ENH: allow for custom patch artist and labels to the boxplots

04937b5

WIP: more tests and add option to toggle box

88435bc

TST: added tests for all of bxp and more boxplot tests TST: fixed boxplot baseline image TST: added baseline images for bxp

DOC/REF: added docstring for axes.bxp and refactored all references t…

1002913

…o outliers as fliers

REF/TST: update boxplot_stats test with outliers changed to fliers

614736d

ENH/TST: boxplots stats now takes a labels kwarg MNT: minor PEP8 whitespace fix

MNT: reran boilerplate.py

ec12ef9

DOC: added blurb in changelog

ffdbd7d

MNT: got rid of build-breaking u'' strings in pyplot

987893c

MNT: fixed changelog whitespace

2562b68

phobson mentioned this pull request Dec 3, 2013

Boxplot: allow whiskers to always cover entire range #1455

Closed

phobson added 5 commits December 3, 2013 13:21

ENH: cbook.boxplot_stats can now set the whiskers to -range- (min/max…

7f937f9

…) or specified percentiles. If IQR is zero, min/max are used

TST: added tests and baseline images for new whisker behavior

fc80458

ENH: added remaining bxp kwargs to boxplot

85d91df

DOC: expanded boxplot_stats docstring

d079ad2

DOC: added examples

5a9f610

tacaswell reviewed Dec 13, 2013
View reviewed changes

phobson mentioned this pull request Dec 13, 2013

ENH: Add ohlc / candlestick plotting option for DataFrame pandas-dev/pandas#783

Closed

DOC: impoved example boxplot figure layout and confirmed that they bu…

13937e9

…ild with docs

efiring reviewed Jan 18, 2014
View reviewed changes

Comment thread doc/users/whats_new.rst Outdated

efiring Jan 18, 2014

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

Maybe "each of the artists"? This is still not very clear.

phobson added 5 commits January 18, 2014 16:27

cleanup up typos and minor tweaks based on dev team input

da97eab

cleaned up whats_new (thanks, efiring)

437f5c2

better shaping checking logic

f26a402

boxplot and bxp docstrings to a more numpy-like format

f2aea5b

fixed usermedian shape logic (again)

9b0118f

tacaswell mentioned this pull request Jan 22, 2014

boxplot treats iterables differently by type #2539

Closed

reran boilerplate.py

64b1196

tacaswell added a commit that referenced this pull request Jan 24, 2014

Merge pull request #2643 from phobson/refactor-boxplots

cc6ae3d

ENH/REF: Overhauled boxplots

tacaswell merged commit cc6ae3d into matplotlib:master Jan 24, 2014

phobson deleted the refactor-boxplots branch January 24, 2014 06:33

takluyver mentioned this pull request Jan 25, 2014

Docs build failure #2766

Closed

This was referenced Jan 27, 2014

New features for boxplot #217

Closed

Possible bug in boxplot() #1027

Closed

tacaswell mentioned this pull request Jan 25, 2015

Hist Plot Normalization should allow a 'Per Bin' Normalization #4038

Closed

QuLogic mentioned this pull request Feb 10, 2018

boxplot regression testing #3572

Closed

has2k1 mentioned this pull request Aug 3, 2019

Implementing geom_hex has2k1/plotnine#296

Open

Conversation

phobson commented Dec 3, 2013

In a nut shell here is what has happened:

Potential remaining items:

This PR addresses the following issue (or allows users to address them themselves):

Uh oh!

tacaswell commented Dec 3, 2013

Uh oh!

phobson commented Dec 3, 2013

Uh oh!

tacaswell Dec 13, 2013

Choose a reason for hiding this comment

Uh oh!

phobson Dec 13, 2013

Choose a reason for hiding this comment

Uh oh!

efiring Jan 19, 2014

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Dec 13, 2013

Uh oh!

phobson commented Dec 13, 2013

Uh oh!

tacaswell commented Dec 13, 2013

Uh oh!

phobson commented Dec 13, 2013

Uh oh!

tacaswell commented Dec 13, 2013

Uh oh!

phobson commented Jan 2, 2014

Uh oh!

tacaswell commented Jan 2, 2014

Uh oh!

phobson commented Jan 2, 2014

Uh oh!

efiring Jan 18, 2014

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Jan 24, 2014

Uh oh!

phobson commented Jan 24, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants