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.

Sign up for GitHub

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

Jump to bottom

StatisticsCleanup: stats.kruskal review #5126

Merged
merged 2 commits into from Aug 9, 2015
Merged

StatisticsCleanup: stats.kruskal review #5126

merged 2 commits into from Aug 9, 2015

Conversation

aeklant
Copy link
Contributor

@aeklant aeklant commented Aug 8, 2015

closes gh-639

@aeklant
MAINT: stats.kruskal code and docs housekeeping.
8c208df 
Added empty input handling and tidied up the code.
Documentation was improved and examples added.

closes scipygh-639
@aeklant
Copy link
Contributor Author

aeklant commented Aug 8, 2015

nan_policy addition is missing. I will have to rebase anyway but I will wait to see which of the PRs merges first (this one or the _chk_asarray one, where the nan_policy implementation is being done) and then rebase and put it in the second one.

Code changes:

  • Added empty input handling to return np.nan, np.nan if any input is empty.
  • Removed unhelpful comments.
  • Changed the name of variable na to num_groups for clarity.

Documentation changes:

  • Added reference to the original paper.
  • Added mannwhitneyu, f_oneway and friedmanchisquare to See Also section.
  • Added two examples (taken directly from the regression tests).

ping @rgommers

@aeklant aeklant force-pushed the kruskal branch 2 times, most recently from a32dc8e to 4b432d3 Compare August 8, 2015 19:21
larsmans
larsmans reviewed Aug 9, 2015
View reviewed changes
scipy/stats/tests/test_stats.py
x = [1, 1, 1]
y = [2, 2, 2]
z = []
h, p = stats.kruskal(x, y, z)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused results.

@rgommers rgommers added scipy.stats maintenance Items related to regular maintenance tasks labels Aug 9, 2015
rgommers
rgommers reviewed Aug 9, 2015
View reviewed changes
scipy/stats/stats.py
@@ -4638,33 +4644,55 @@ def kruskal(*args):

References
----------
.. [1] http://en.wikipedia.org/wiki/Kruskal-Wallis_one-way_analysis_of_variance
.. [1] Kruskal, William H. & Wallis, W. Allen, "Use of Ranks in
Copy link
Member

Choose a reason for hiding this comment

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

Can you write the names as W.H. Kruskal and W.W. Allen?

Copy link
Member

Choose a reason for hiding this comment

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

And Volume as Vol.? Both minor, but good to keep things consistent with the numpy docstring guidelines.

@rgommers
Copy link
Member

rgommers commented Aug 9, 2015

Looks good to me, can be merged once the minor comments are addressed.

@aeklant
Copy link
Contributor Author

aeklant commented Aug 9, 2015

Comments addressed. Nothing major so TravisCI should pass again.

@rgommers rgommers added this to the 0.17.0 milestone Aug 9, 2015
rgommers added a commit that referenced this pull request Aug 9, 2015
@rgommers
Merge pull request #5126 from aeklant/kruskal
ac598a1 
MAINT: StatisticsCleanup: `stats.kruskal` review
@rgommers rgommers merged commit ac598a1 into scipy:master Aug 9, 2015
@rgommers
Copy link
Member

rgommers commented Aug 9, 2015

Green & merged. Thanks @aeklant

@aeklant aeklant deleted the kruskal branch August 9, 2015 19:03
@aeklant aeklant mentioned this pull request Aug 13, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
maintenance Items related to regular maintenance tasks scipy.stats
Projects
None yet
Milestone
0.17.0
Development

Successfully merging this pull request may close these issues.

Statistics Review: kruskal (Trac #112)
3 participants
@aeklant @rgommers @larsmans