New boxplot features - #1199

So, for my sanity. I rolled back axes.py to before that email in #217. The corresponding commit is d0c4100. I pulled out the boxplot method from that commit, let's call it old_boxplot_method.py. This file shouldn't contain any of the changes proposed in #217. However the diff between old_boxplot_method.py and new_boxplot_method.py is empty...

Any ideas?

This pull request fails (merged 29bbc8b into a7aaa83).

This is getting more and more confusing. I take it you found the original diff that was posted to the mailing list. Maybe it would help if you put that in a gist so we can see it. With regard to your finding no diff between your old and new versions, is it possible that in generating one of them you applied a patch in the wrong direction? Maybe the original diff was generated in the wrong direction?

Overall, the only way to take advantage of the original diff is likely to be to understand what it was doing, see which of things have not already been done, and then re-implement them manually in current code. The most obvious and easy of these is the option to include a mark for the mean. Maybe the thing to do is start with that as a single PR, if this is indeed something that boxplot users would benefit from. This is far from clear; a quick look at R documentation and wikipedia does not show any evidence that a mean mark is a normal part of a boxplot. I don't use boxplots, so I can't comment further.

Are you sure you want to pursue this one...

@efiring Yes, I know. I've contacted the original author for confirmation/clarification regarding his suggestions. I'll follow up if I hear anything.

Am I sure I want to pursue this? Not at all. I'm just trying to help close some of the really old issues.

On 2012/09/03 10:59 AM, Damon McDougall wrote:

Am I sure I want to pursue this? Not at all. I'm just trying to help
close some of the really old issues.
And that is very much appreciated. Having all those old issues
sitting around is unhealthy, but sometimes it is hard to know what to do
with them. You and @pelson are doing a wonderful job of figuring it out.

I'm sure y'all are really busy with the upcoming release. I wouldn't mind volunteering to get this is better form as I'm pretty familiar with the boxplot function these days.

I could also incorporate these changes with yet another new feature I cooked up yesterday:
phobson@5fd8308

@pelson - I'm conflicted b/c if I had thought of the idea linked above, I never would have added the usermedians and conf_intervals kwargs and now i feel like boxplot is getting pretty bloated :/

@phobson Awesome! Help is greatly appreciated. I haven't looked at your suggested feature yet.

I have touched base with the author, Rob Clewley, who has kindly provided the diff and sample output. His email to me follows:

Hi Damon,

I don't know if it's helpful but here's my diff output attached from
what I believe was v0.90 of matplotlib (back in 2007), as well as my
versions of the files involved. If you can't make any of this work, I
could take a look at the current release version of the boxplot method
and maybe re-doctor it?

Let me know,
Rob

Here are the attachments:

diff_output.txt: https://gist.github.com/3736698
axes (boxplot method).py: https://gist.github.com/3736702
new_boxplot_metod.py: https://gist.github.com/3736707

Sample output:

Rob's original nabble email is here: https://gist.github.com/3736715

The next steps are to go through his email, figure out what functionality does/does not need to be added. Once that is done, go through the features that need to be added and give a green light if the feature is desired/sensible. The last step, in my opinion, is then to create a PR for each new feature and critique the change.

@dmcdougall
Thanks for the resources. I'm happy to help. Is it safe to assume that since v1.2 is in the RC stage, that there's not much a rush to get this done? (pretty hectic at work this week)

@phobson You're welcome. Nothing that constitutes a new feature will be merged into the v1.2 branch. Since everybody's effort is currently on creating a stable 1.2 release, there is no rush to resolve this right now. Thanks again for offering to help, it's great to get feedback from someone that will use newly implemented features.

@dmcdougall @pelson I started work on this last night. I'm doing each feature in a separate branch. I'm keeping (public) track of my progress here:
https://gist.github.com/3747864

Since 1 PR per feature will mean I'll have to merge and rebase as things get accepted, I've listed things in the order in which I would like them to be merged into master (should that actually happen).

I'll continue to chip away at this, but I'll keep the fire hose shut off until v1.2 is out in the wild.

@phobson I see you are making good progress. At some point I will fork your stuff and try it out :)

@dmcdougall & @phobson - The boxplot code has changed significantly in the last 8 months. Is this PR still relevant? Have the desired features from this PR already been applied?

Cheers,

@pelson the short answer is that the PR is probably not relevant anymore and that the desired functionality has not been implemented.

I'm tempted to abandon this effort and approach the situation a different way:
Just like R, I think we should have two boxplot functions. The first computes all of the positions of the boxes' features and the feeds that data to a second function that actually does the drawing. If that second function is publicly available, that'll give the users the freedom to do absolutely whatever the the very original feature request desired and with a lot less code/kwargs/logic/etc.

I'll be happy to cut a branch and do that refactoring. It's probably obvious, but that won't happen before the 1.3 release.

Thanks @phobson. I'll close this and when we (as users) need the functionality, we can look into implementing it then.