Skip to content
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

ENH: rewrite of stats.ppcc_plot #4563

Merged
merged 5 commits into from
Apr 4, 2015
Merged

ENH: rewrite of stats.ppcc_plot #4563

merged 5 commits into from
Apr 4, 2015

Conversation

rgommers
Copy link
Member

Closes Statistics Review issue gh-679.

Also some minor fixes to stats.probplot and stats.tukeylambda are included.

@rgommers rgommers added scipy.stats enhancement A new feature or improvement labels Feb 26, 2015
@rgommers
Copy link
Member Author

rgommers commented Apr 4, 2015

This PR should be pretty straightforward, any takers for a review?

@WarrenWeckesser
Copy link
Member

I haven't reviewed the ppcc_plot changes, but I noticed that the tukeylambda PPF function could be simplified to boxcox(q, lam) - boxcox1p(-q, lam), where boxcox and boxcox1p are from scipy.special. If you'd like, you can remove your change to tukeylambda._ppf, and I'll create a PR to make this change.

@josef-pkt
Copy link
Member

Reading through the code: The changes look good to me, I don't see any problems and it improves the code.

I haven't paid any attention to what the empty array behavior is now in various functions.
I guess if there is only one point, or maybe a few, the plot and fit will break down. Is probplot defined for very small arrays?

@rgommers
Copy link
Member Author

rgommers commented Apr 4, 2015

probplot seems to work for any length input:

  • length 0 is special cased
  • length 1 gives a RuntimeWarning and returns ((array([ 0.]), array([1])), (nan, nan, 0.0)).
  • length 2 and up return values and don't give any warning. There's not much point of course for a length-2 input, but it seems overkill to add warnings for specific input sized there (also not clear where you'd put the cutoff then).

Plots are all sensible as well, except a plot for length-1 which of course is just a single point in a figure.

For empty input ppcc_plot simple inherits the probplot behavior. At some point we may want to go through and review empty input handling for consistency between all stats functions, but that's not something I want to do now or in the near future.

@rgommers
Copy link
Member Author

rgommers commented Apr 4, 2015

@WarrenWeckesser that sounds good. Although easier, why not merge this PR now (since Josef just reviewed it) and then you make your proposed change on top of it?

@WarrenWeckesser
Copy link
Member

@rgommers: That's fine, too--less work for you, which is good. :)

rgommers added a commit that referenced this pull request Apr 4, 2015
ENH: rewrite of stats.ppcc_plot
@rgommers rgommers merged commit e22f52a into scipy:master Apr 4, 2015
@rgommers
Copy link
Member Author

rgommers commented Apr 4, 2015

Great, thanks. Merging.

@rgommers rgommers deleted the fixup-ppcc branch April 4, 2015 16:07
@WarrenWeckesser
Copy link
Member

tukeylambda ppf change is here: #4692

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants