Skip to content

Boxplot stats w/ equal quartiles #5343

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.

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

Merged
merged 15 commits into from
Mar 14, 2016

Conversation

phobson
Copy link
Member

@phobson phobson commented Oct 29, 2015

See #5331

Addresses the concern raised in the issue above and cleans up the docstring.

Previous behavior available through the autorange kwarg.

def boxplot_stats(X, whis=1.5, autorange=False, bootstrap=None,
labels=None):
"""
Returns list of dictionaries of staticists to be use to draw a
Copy link
Member

Choose a reason for hiding this comment

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

sp. statistics

"to be use" -> "to use"

@mdboom
Copy link
Member

mdboom commented Oct 29, 2015

Cool. Much improved. 👍

@phobson
Copy link
Member Author

phobson commented Oct 29, 2015

@mdboom thanks for the comments. I think I got them all as they came in 😄

...aaaaaand here's the part where I pitch the idea that we add in an option to pass your own bootstrapper.

Something like:

def boxplots_stats(..., bootstrap_fxn=None):
    if bootstrap_fxn is None:
        bootstrap_fxn = _bootstrap_median

    # ...
    def _compute_conf_interval(data, med, iqr, bootstrap):
        if bootstrap is not None:
            # Do a bootstrap estimate of notch locations.
            # get conf. intervals around median
            CI = bootstrap_fxn(data, N=bootstrap)
            notch_min = CI[0]
            notch_max = CI[1]
        else:
            N = len(data)
            notch_min = med - 1.57 * iqr / np.sqrt(N)
            notch_max = med + 1.57 * iqr / np.sqrt(N)

    # yada yada 

@phobson phobson force-pushed the bxp-equal-quartiles branch from 9c9d7ad to d9677e9 Compare October 29, 2015 01:39
@tacaswell
Copy link
Member

I am starting to think that box plots are a complicated enough topic that they should be spun off into a sub-project (which is allowed to do things like require pandas and scipy). Probably take violin plots with them too.

@QuLogic
Copy link
Member

QuLogic commented Oct 29, 2015

Isn't that essentially seaborn?

@phobson phobson force-pushed the bxp-equal-quartiles branch from ffe9760 to fc288e4 Compare October 29, 2015 06:15
@WeatherGod
Copy link
Member

We would also still want basic boxplot/violinplot functionality for those
who do not need to delve into pandas or seaborn.

On Wed, Oct 28, 2015 at 10:28 PM, Elliott Sales de Andrade <
notifications@github.com> wrote:

Isn't that essentially seaborn?


Reply to this email directly or view it on GitHub
#5343 (comment)
.

@phobson phobson force-pushed the bxp-equal-quartiles branch 2 times, most recently from 9bde579 to ec2eeab Compare October 29, 2015 18:03
@phobson phobson force-pushed the bxp-equal-quartiles branch from ec2eeab to 0d8db73 Compare November 20, 2015 19:11
@phobson
Copy link
Member Author

phobson commented Nov 20, 2015

rebased with current master (branch had gotten stale)

75th percentiles are equal, ``whis`` is set to ``'range'`` such
that the whisker ends are at the minimum and maximum of the
data.
meanline : bool, optional (False)
Copy link
Member

Choose a reason for hiding this comment

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

the meanline docs should move back up to be in the right order to match the signature.

Copy link
Member

Choose a reason for hiding this comment

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

eh, maybe not, I can go either way on this on further consideration.

@phobson phobson force-pushed the bxp-equal-quartiles branch from 761ee40 to ae398d2 Compare November 24, 2015 21:02
@tacaswell tacaswell added this to the 1.5.2 (Critical bug fix release) milestone Jan 26, 2016
@tacaswell
Copy link
Member

tagged this as a bug fix, but I am not sure if that is the correct tag for this.

@phobson
Copy link
Member Author

phobson commented Jan 26, 2016

@tacaswell should I rebase on 1.5.X or 2?

@tacaswell
Copy link
Member

on to master then we will back-port the merge to where ever we decide this will go.

@phobson phobson force-pushed the bxp-equal-quartiles branch from ae398d2 to 7b044c0 Compare January 26, 2016 17:48
@QuLogic
Copy link
Member

QuLogic commented Jan 26, 2016

It looks like you committed the conflicts in the SVG files:

.../result_images/test_axes/boxplot_autorange_false_whiskers-expected.svg:470: parser error : StartTag: invalid element name
<<<<<<< HEAD
    ^

@phobson
Copy link
Member Author

phobson commented Feb 18, 2016

Bump -- just gave this another rebase to keep it current with master

@phobson phobson force-pushed the bxp-equal-quartiles branch from efa39e7 to 14192ca Compare February 18, 2016 04:27
@phobson
Copy link
Member Author

phobson commented Feb 18, 2016

Build failures are unrelated. Something's wacky with colorbars, e.g.,
FAIL: matplotlib.tests.test_colorbar.test_colorbar_closed_patch.test

@WeatherGod
Copy link
Member

Yes, this came about due to a merge of a different PR last night, I think.

On Thu, Feb 18, 2016 at 12:16 PM, Paul Hobson notifications@github.com
wrote:

Build failures are unrelated. Something's wacky with colorbars, e.g.,
FAIL: matplotlib.tests.test_colorbar.test_colorbar_closed_patch.test


Reply to this email directly or view it on GitHub
#5343 (comment)
.

@tacaswell
Copy link
Member

Sorry, I broke all the branches. Merged a PR that passed before we put the zero-tolerance in. There were some regions of those images where the 8bit blue value changed by 1

@jenshnielsen
Copy link
Member

The failure on appveyor is

FAIL: Github issue #1256 identified a bug in Line.draw method 
---------------------------------------------------------------------- 
Traceback (most recent call last):
  File "C:\conda\envs\test-environment\lib\site-packages\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "c:\projects\matplotlib\lib\matplotlib\testing\decorators.py", line 152, in wrapped_callable
    func(*args, **kwargs)
  File "c:\projects\matplotlib\lib\matplotlib\tests\test_lines.py", line 61, in test_invisible_Line_rendering
    assert_true(slowdown_factor < slowdown_threshold)
AssertionError: False is not true 

which is known to be flaky

@phobson
Copy link
Member Author

phobson commented Mar 8, 2016

Bump -- give me a shout if y'all want any changes made to this.

keys of the dictionary. Users can skip this function and pass a user-
defined set of dictionaries to the new `axes.bxp` method instead of
relying on MPL to do the calcs.
def boxplot_stats(X, whis=1.5, autorange=False, bootstrap=None,
Copy link
Member

Choose a reason for hiding this comment

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

Is this considered a public function? If so the new arg needs to go last.

@tacaswell
Copy link
Member

Read through this, other than my one comment 👍.

The docstrings are much better.

Can this get a note in api_changes? To check my understanding:

  • this is a minor API change (ex the plot will be different for same inputs) but
    1. only in semi-pathological edge case
    2. the change will make edge case more consistent with normal case
    3. the way it was is considered confusing by those skilled in the art of boxplots
    4. there is a way to get the old plots back

@phobson
Copy link
Member Author

phobson commented Mar 13, 2016

@tacaswell your understanding matches mine.

and to make sure I'm clear -- I don't modify api_changes.rstdirectly, I just add a short, single file: api_changes/2016-03-12-PMH.rst?

@tacaswell
Copy link
Member

Yes. The individual files helps prevent rebase-due-to-doc-conflicts
thrashing.

On Sun, Mar 13, 2016 at 6:41 PM Paul Hobson notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell your understanding matches mine.

and to make sure I'm clear -- I don't modify api_changes.rstdirectly, I
just add a short, single file: api_changes/2016-03-12-PMH.rst?


Reply to this email directly or view it on GitHub
#5343 (comment)
.

@phobson phobson force-pushed the bxp-equal-quartiles branch from 3743f6b to 6fe8a72 Compare March 14, 2016 06:43
jenshnielsen added a commit that referenced this pull request Mar 14, 2016
@jenshnielsen jenshnielsen merged commit 455cb92 into matplotlib:master Mar 14, 2016
jenshnielsen added a commit to jenshnielsen/matplotlib that referenced this pull request Mar 14, 2016
@jenshnielsen
Copy link
Member

Backport wasn't clean (conflict in removed svg file) so doing it via #6153

jenshnielsen added a commit that referenced this pull request Mar 14, 2016
Backport #5343 from phobson/bxp-equal-quartiles
@phobson phobson deleted the bxp-equal-quartiles branch March 14, 2016 15:53
@phobson
Copy link
Member Author

phobson commented Mar 14, 2016

thanks for the merge and help, everyone!

tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request May 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants