D21977: One Way Anova Test

[Stefan Gerlach]

sgerlach added inline comments.

INLINE COMMENTS

> HypothesisTest.cpp:471
> +void HypothesisTestPrivate::performOneWayAnova() {
> +    // all standard variables and formulas are taken from this wikipedia page:
> +    // https://en.wikipedia.org/wiki/One-way_analysis_of_variance

please put comments describing the function before the function. Take a look at what doxygen expects.

> HypothesisTest.cpp:476
> +    // w stands for within groups
> +    clearGlobalVariables();
> +    int np, total_rows;

do we have global variables? If yes, why do we need them?
Avoiding global vars is a good programming practise

> HypothesisTest.cpp:489
> +
> +    double y_bar = 0;
> +    double s_b = 0;

can you comment the variables? The names are not always self-explanatory (at least for me).

> HypothesisTest.cpp:506
> +    for (int i = 0; i < np; i++) {
> +        s_b += ni[i] * qPow( ( mean[i] - y_bar), 2);
> +        if (ni[i] > 1)

how efficient is qPow()? Does it make sense to use a gsl function for small inter powers to get better performance? See https://www.gnu.org/software/gsl/doc/html/math.html#small-integer-powers

> HypothesisTest.cpp:544
> +
> +    m_stats_table = "<h3>Group Summary Statistics</h3>";
> +

Can the following strings be made translatable? If yes, please do so.

> HypothesisTest.cpp:978
>          case HypothesisTest::TailTwo:
> -            p_value = nsl_stats_tdist_p(value, df) + nsl_stats_tdist_p(-1*value, df);
> +            p_value = 2.*gsl_cdf_tdist_P(value, df);
>  

this is only correct, when the distribution is symmetric? Is this true for the t-distribution?

> HypothesisTest.cpp:994
>              value *= -1;
> -            p_value = nsl_stats_tdist_p(value, df);
> +            p_value = nsl_stats_tdist_p(value - mean, sp);
>              printLine(0, i18n("Null Hypothesis: Population mean of %1 %2 Population mean of %3 ", col1_name, UTF8_QSTRING("≤"), col2_name), "blue");

not gsl_cdf_gaussian_P?

> HypothesisTestDock.cpp:77
> +
> +    test_type_t_z.append("Two Sample Independent");
> +    test_type_t_z.append("Two Sample Paired");

if the following strings are user visible please add "i18n()".

> HypothesisTestDock.cpp:253
> +void HypothesisTestDock::showTestType() {
>      ttest = ui.cbTest->currentText() == "T Test";
>      ztest = ui.cbTest->currentText() == "Z Test";

please use cbTest->currentItem() and a matching enum (see above)

> HypothesisTestDock.h:77
>      bool two_sample_independent{false};
>      bool two_sample_paired{false};

can we collect all the bool vars in an enum or a bit-field. Do we even need all of them (one_way=false is equal to two_way=true and vice-versa)?

REPOSITORY
  R262 LabPlot

REVISION DETAIL
  https://phabricator.kde.org/D21977

To: devanshuagarwal, sgerlach, asemke
Cc: kde-edu, #labplot, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190622/8b125378/attachment-0001.html>