User-specified medians and conf. intervals in boxplots #906

phobson · 2012-05-27T16:36:59Z

First, here's a test script: https://gist.github.com/2814818

I know I've submitted at least one other PR for this, but this time I've think I've got it right (or at least much closer).

Basically, the user provides a list of medians and confidence intervals where (if using numpy arrays)
usermedians.shape = (N,)
conf_intervals.shape = (N,2)

and the data to be plotted has: data.shape = (M,N).

All of this allows the user to compute the medians and its confidence intervals outside of the boxplot function using more statistically robust methods of his or her choice.

I've used a lot of assert statements to verify that the usermedians and conf_intervals inputs are compatible with the data being plotted (x in the axes.boxplot call signature).

Hope this is useful and can be incorporated into the library.

Thanks for all of the hard work
-paul

pelson · 2012-05-29T14:33:28Z

I've been thinking of implementing something similar recently, but you've beat me to it ;-)

I notice your spacing is not looking so hot. What editor are you using? Does it put tabs in instead of 4 spaces? Would it be easy for you be able to correct this before we go much further in the review?

phobson · 2012-05-29T15:29:56Z

Yikes! Yes. I'll fix that. I cooked up a quick VM at home this weekend
-- left the work machine at home :-) -- and it looks like i didn't get
my vimrc set up right.

Thanks for looking at the PR.
-paul

On Tue, May 29, 2012 at 7:33 AM, Phil Elson
reply@reply.github.com
wrote:

I've been thinking of implementing something similar recently, but you've beat me to it ;-)

I notice your spacing is not looking so hot. What editor are you using? Does it put tabs in instead of 4 spaces? Would it be easy for you be able to correct this before we go much further in the review?

Reply to this email directly or view it on GitHub:
#906 (comment)

phobson · 2012-05-29T16:35:03Z

OK. This is looking good on my system now. Rebuilt matplotlib from scratch, ran my script, and everything worked as expected.

phobson · 2012-05-29T16:39:46Z

lib/matplotlib/axes.py

-                            bsIndex = np.random.random_integers(0,M-1,M)
-                            bsData = data[bsIndex]
-                            estimate[n] = mlab.prctile(bsData, 50)
-                        CI = mlab.prctile(estimate, percentile)


Just occurred to me that we should use numpy.percentile now (assuming it's available in minimum version of numy that MPL supports.

mdboom · 2012-06-01T14:19:51Z

This looks good. Can you add an example and a unit test?

pelson · 2012-06-15T12:41:25Z

examples/pylab_examples/boxplot_demo3.py

 text_transform= mtransforms.blended_transform_factory(ax.transData,
                                                     ax.transAxes)
 ax.set_xlabel('treatment')
 ax.set_ylabel('response')
-ax.set_ylim(-0.2, 1.4)
+#ax.set_ylim(-0.2, 1.4)


This comment serves no purpose other than to make it harder to read. Would you mind just nuking it?

pelson · 2012-06-15T12:57:10Z

Would you mind adding a simple unit test?

Other than that, this change gets my thumbs up. Great work!

phobson · 2012-06-15T14:00:56Z

Phil, I will happily address all of these comments shortly. It has been a
hectic two weeks at the office and I'm on vacation shortly. Thank you so
much for looking at the PR.

On Friday, June 15, 2012, Phil Elson wrote:

Would you mind adding a simple unit test?

Other than that, this change gets my thumbs up. Great work!

Reply to this email directly or view it on GitHub:
#906 (comment)

pelson · 2012-06-15T14:05:01Z

@phobson: No problems. Have a great break!

phobson · 2012-07-15T19:38:34Z

@pelson

I addressed your comments. Sorry for the delay. In summary:

switch the vert/notch kwargs over to true/false (though 1/0 still work as in the demos) and updated the docstring to reflect this
combined my conf_intervals if-logic into a single statement and removed the lines that became extraneous as a result
added a unit test + baseline images
modified a demo to show this functionality
enhanced the docs about the dictionary that is returned

WeatherGod · 2012-07-21T15:54:45Z

@pelson, has the OP addressed your concerns here?

WeatherGod · 2012-07-21T15:58:46Z

lib/matplotlib/axes.py

+        msg2 = "usermedians' length must be compatible with x"
+        if usermedians is not None:
+            if hasattr(usermedians, 'shape'):
+                assert len(usermedians.shape) == 1, msg1


Don't do asserts for checking user inputs. Raise a ValueError instead.

WeatherGod · 2012-07-21T16:02:01Z

Neat work. We are close to getting this merged in. One more very important thing to add is a note in doc/users/whats_new.rst. Also, you need to run boilerplate.py in the matplotlib's source directory in order to regenerate the pyplot file.

pelson · 2012-07-22T12:55:12Z

lib/matplotlib/axes.py

+                # conf. intervals from user, if available
+                if conf_intervals is not None and \
+                   conf_intervals[i] is not None:
+		    notch_max = np.max(conf_intervals[i])


This indentation looks a little funny. Have tabs been used here?

I think it looked a little funny since the conditional was wrapped around the second line. python parsed it fine, but i've lined everything up to look nicer. will see with my next commit/push

pelson · 2012-07-22T12:57:43Z

@pelson, has the OP addressed your concerns here?

Yes. I am finding it hard to assert that the logic/statistics are identical (thanks to the improvement of layout). But what I can see looks good. Gets my +1.

WeatherGod · 2012-07-22T15:04:05Z

@pelson, good to know. Tasks that remain: get pyplot.py regenerated, odd indentation fixed (or explained), and the new entry to the whats_new.rst,

phobson · 2012-07-22T18:09:58Z

@WeatherGod Sorry to require so much hand-holding, here. I ran boilerplate.py like this:
paul@flint ~/sources/matplotlib $ python boilerplate.py
[prints lots of stuff to terminal, then...]

paul@flint ~/sources/matplotlib $ git status

On branch manual-boxplots-2

nothing to commit (working directory clean)
paul@flint ~/sources/matplotlib $

There don't seem to be any calls to sys.argv, so i'm not sure how to get this to behave properly. Any advice would be much appreciated. Thanks.

pelson · 2012-07-22T20:38:04Z

It may be that your branch is based on a version before the bolierplate script was updated. My advice at this stage would be to rebase your branch (the rebase is needed anyway), and then run the boilerplate.py script. The workflow goes something like:

git checkout manual-boxplots-2
git fetch upstream
git rebase upstream/master
# Fix conflicts, if any
git push -f origin manual-boxplots-2

…ng boxplots

…e confidence intervals

…g anything and there's no point in adjusting the subplot spacing.

…'s request

phobson · 2012-07-23T06:15:17Z

@pelson thanks for the git-fu! worked like a charm.

WeatherGod · 2012-07-23T13:18:14Z

lib/matplotlib/tests/test_axes.py

@@ -670,6 +670,7 @@ def test_hist_log():
    ax.set_xticks([])
    ax.set_yticks([])

+<<<<<<< HEAD


Looks like you missed something while resolving conflicts. This means you didn't run the tests after rebasing. Please run the tests to make sure everything looks good. This may also impact the resulting test images because changes may have occurred to the rendering algorithms (line snapping, text anti-aliasing, etc.).

@WeatherGod: Just cleared that up. Sorry for the slow response. All tests just passed.

pelson · 2012-08-11T21:40:18Z

I've just merged this by hand (447a7cc), squashing the commits and resolving the remaining conflicts.

@phobson: Thanks for doing this work! If you have any more enhancements, I would be more than happy to review them!

phobson · 2012-08-16T20:22:56Z

@pelson Thanks for squashing and merging! Really glad I could contribute back to matplotlib! I also really appreciate the feedback and guidance through the whole process from you and everyone else.

phobson reviewed May 29, 2012
View reviewed changes

pelson reviewed Jun 15, 2012
View reviewed changes

WeatherGod reviewed Jul 21, 2012
View reviewed changes

pelson reviewed Jul 22, 2012
View reviewed changes

phobson added 7 commits July 22, 2012 23:04

users can specify the median and it's confidence interval when creati…

c96118f

…ng boxplots

added assert messages to help user

6491ce0

fixed embarrassing tabs vs 4spaces

4328635

fixed bad indent on 1 line in axes.boxplot

1767e28

formatted (indented) the docstring os axes.boxplot

a0d30f5

weird text issue - accidentally copied text from other buffer

146a6e2

tried to standardized arg/kwarg doc format in axes.boxplot

9efe05a

phobson added 14 commits July 22, 2012 23:04

added in "function arguments" header

ae69ae1

modified an example to include the new functionality

45711e5

minor tweaks to clean up my boxplot example and the logic handling th…

ba8890a

…e confidence intervals

Got rid of a lot cruft in the example. the transform line wasn't doin…

88d50ce

…g anything and there's no point in adjusting the subplot spacing.

no more subplot adjusting

1c79eb8

added test for new boxplot functionality

5464e34

fixed my data construction with np.hstack

ffc8345

np.hstack/np.linspace errors

d3b3d5c

added baseline images and unit test for new boxplot functionality

e6403b2

switched notch and vert kwargs over to True/False instead of 0 or 1

c052d07

switched from assert statements to raiseing value errors per ben root…

3db645a

…'s request

cleaned up ambiguous indentation around a multiline if statement

e157dd1

added entry to what's new

3172ef2

reran boilerplate.py to update pyplot

0b65488

WeatherGod reviewed Jul 23, 2012
View reviewed changes

fixed remaining conflict in test_axes.py

a2085a6

pelson closed this Aug 11, 2012

phobson deleted the manual-boxplots-2 branch January 27, 2014 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User-specified medians and conf. intervals in boxplots #906

User-specified medians and conf. intervals in boxplots #906

phobson commented May 27, 2012

pelson commented May 29, 2012

phobson commented May 29, 2012

phobson commented May 29, 2012

phobson May 29, 2012

mdboom commented Jun 1, 2012

pelson Jun 15, 2012

pelson commented Jun 15, 2012

phobson commented Jun 15, 2012

pelson commented Jun 15, 2012

phobson commented Jul 15, 2012

WeatherGod commented Jul 21, 2012

WeatherGod Jul 21, 2012

WeatherGod commented Jul 21, 2012

pelson Jul 22, 2012

phobson Jul 22, 2012

pelson commented Jul 22, 2012

WeatherGod commented Jul 22, 2012

phobson commented Jul 22, 2012

pelson commented Jul 22, 2012

phobson commented Jul 23, 2012

WeatherGod Jul 23, 2012

phobson Jul 24, 2012

pelson commented Aug 11, 2012

phobson commented Aug 16, 2012

User-specified medians and conf. intervals in boxplots #906

User-specified medians and conf. intervals in boxplots #906

Conversation

phobson commented May 27, 2012

pelson commented May 29, 2012

phobson commented May 29, 2012

phobson commented May 29, 2012

phobson May 29, 2012

Choose a reason for hiding this comment

mdboom commented Jun 1, 2012

pelson Jun 15, 2012

Choose a reason for hiding this comment

pelson commented Jun 15, 2012

phobson commented Jun 15, 2012

pelson commented Jun 15, 2012

phobson commented Jul 15, 2012

WeatherGod commented Jul 21, 2012

WeatherGod Jul 21, 2012

Choose a reason for hiding this comment

WeatherGod commented Jul 21, 2012

pelson Jul 22, 2012

Choose a reason for hiding this comment

phobson Jul 22, 2012

Choose a reason for hiding this comment

pelson commented Jul 22, 2012

WeatherGod commented Jul 22, 2012

phobson commented Jul 22, 2012

On branch manual-boxplots-2

pelson commented Jul 22, 2012

phobson commented Jul 23, 2012

WeatherGod Jul 23, 2012

Choose a reason for hiding this comment

phobson Jul 24, 2012

Choose a reason for hiding this comment

pelson commented Aug 11, 2012

phobson commented Aug 16, 2012