-
Notifications
You must be signed in to change notification settings - Fork 74
Add Anova #62
base: master
Are you sure you want to change the base?
Add Anova #62
Conversation
trevismd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit late, but some suggestions, in case the author finds the time to review/merge PRs.
I'm not a maintainer of this repository, though.
| pval, | ||
| ) | ||
| elif test == 'anova': | ||
| u_stat, pval = stats.f_oneway( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clarity in the code, and accurate reporting below, I'd change u_stat to f_stat.
| u_stat, | ||
| pval, | ||
| ) | ||
| elif test == 'anova': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most tests (not t-test_something) are capitalized, so I'd suggest Anova for consistency
| valid_list = ['t-test_ind', 't-test_welch', 't-test_paired', | ||
| 'Mann-Whitney', 'Mann-Whitney-gt', 'Mann-Whitney-ls', | ||
| 'Levene', 'Wilcoxon', 'Kruskal'] | ||
| 'Levene', 'Wilcoxon', 'Kruskal', "anova"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
| EDIT 16.11.2020 - Veronika Brumovska | ||
| - implementing ANOVA (line 84) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this was ever done in this repo, but it would be great to be logged.
I wouldn't put that here though. Maybe we could suggest a changelog or acknowledgement file dedicated to contributions ?
I added Anova test.