-
-
Notifications
You must be signed in to change notification settings - Fork 1k
ANOVA Functionality #148
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
ANOVA Functionality #148
Conversation
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.
Awesome, Frank! Thanks for all your work. I left some comments on things we should resolve before merging in this pull request.
| <!-- /.examples --> | ||
|
|
||
| <section class="links"> | ||
|
|
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.
Please add
[mdn-array]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array
[mdn-typed-array]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Typed_arrays
so that the Markdown reference links are not missing their definitions and are rendered correctly.
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.
Resolved.
| @@ -0,0 +1,140 @@ | |||
| # One Way ANOVA | |||
|
|
|||
| > Perform One-Way ANOVA on given data | |||
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.
Let's change the description to Perform a one-way analysis of variance.
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.
Resolved.
|
|
||
| ### anova1( x, factor\[, ...params]\[, opts] ) | ||
|
|
||
| For an [array][mdn-array] or [typed array][mdn-typed-array] of numeric values `x` and an [array][mdn-array] of classifications `factor`, one-way anova1 is performed. The hypotheses are given as follows: |
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.
Let's revert this undesired change that was introduced when renaming the package: s/anova1/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.
I am unsure what you mean here. I am taking this to mean the removal of the optional parameters that we discussed after the 200 staff meeting on Tuesday. I will ask about this when we meet tomorrow.
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 believe @Planeshifter is referring to the description:
... classifications
factor, one-way anova is performed.
|
|
||
| For an [array][mdn-array] or [typed array][mdn-typed-array] of numeric values `x` and an [array][mdn-array] of classifications `factor`, one-way anova1 is performed. The hypotheses are given as follows: | ||
|
|
||
| $$H_{0}: \mu_{1} = \mu_{2} = \dots = \mu_{k}$$ |
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.
We have a special way of writing formulas so that we can later automatically render the equations using MathJax. For this one, we could do
<!-- <equation class="equation" label="eq:hypotheses" align="center" raw="\begin{align*} H_{0}:& \; \mu_{1} = \mu_{2} = \dots = \mu_{k} \\ H_{a}:& \; \text{at least one} \; \mu_{i} \; \text{not equal to the others} \end{align*}" alt="Hypotheses of ANOVA"> -->
<!-- </equation> -->
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.
Resolved. Out of curiosity, does the markdown format always revert to MathJax?
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.
Yes, as MathJax is what we use for server-side equation rendering. See
| var mathjax = require( 'mathjax-node-sre' ); |
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.
Thanks!
| @@ -0,0 +1,19 @@ | |||
| 'use strict'; | |||
|
|
|||
| var ANOVA = require('../lib/make_anova.js'); | |||
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.
Let's rename ANOVA to anova1 here.
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.
Resolved.
| var newMean; | ||
|
|
||
|
|
||
| if (x.length !== factor.length) { |
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.
It's fine to remove this check given since this function is only called inside of make_anova.js (where we have already established that the arrays have the same length) and won't be publicly available.
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.
Ok. This may be useful for future development yet for now I agree with the sentiment. Removed.
| function meanTable(x, factor) { | ||
| var treatments; | ||
| var factorCount; | ||
| var meanTable; |
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.
Can we rename the meanTable variable? As-is, it has the same name as the function, which can be confusing.
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.
tableOfMeans
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.
You probably don't have to be as descriptive. It would be enough to name the variable out. The meaning of the variable is readily available from the function docs, and, as it stands now, one has to read until function end to know what value is returned from the function.
| 'use strict'; | ||
|
|
||
| /** | ||
| * One-Way Analysis of Variance. |
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.
One-way analysis of variance.
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.
Resolved.
| f = ['frank', 'philipp', 'nugent', 'frank', 'philipp', 'nugent', 'frank', 'philipp', 'nugent', 'frank']; | ||
| out = anova1(x, f); | ||
|
|
||
| t.equal(out.treatment_df, 2); |
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.
Where do these values come from? We should add a comment, e.g. // Tested against R:. Better yet would be to have a script to generate the R fixtures automatically. But we can add that later.
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.
These values came from R testing side by side, forgot to add the R testing fixtures. Will do at a later date.
|
|
||
| // Now to make the out object | ||
| out = {}; | ||
| setReadOnly(out, 'treatment_df', treats.length - 1); |
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.
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 would take inspiration from MATLAB (https://www.mathworks.com/help/stats/anova1.html), including in terms of the output data structures.
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.
@kgryte The MATLAB looks much cleaner than what I have; far more readable. I will update the naming to mirror it. For Prob>F could we simply replace it with p?
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.
Instead of Prob>F, let's use pValue to be consistent with the other stats packages.
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.
Yes, or pValue, whichever is the convention currently used my @Planeshifter elsewhere.
Resolves # .
This pull request will add ANOVA functionality.
Checklist
develop.developbranch.Description
This pull request:
anovafunction to perform one-way ANOVA given an array of continuous values and an array of factors. Support for other input methods will be added.Related Issues
Questions
No.
Other
No.
@stdlib-js/reviewers