One Way Anova Test
AcceptedPublic

Authored by devanshuagarwal on Jun 21 2019, 8:44 PM.

Details

Reviewers
sgerlach
asemke
Summary

I have added one way anova test.
Currently, we can only select one independent column (with multiple levels) and one dependent column.

I have also modified the combo box model for showing columns for tests. Now, QStringList is used as a kind of model for combo box ui.cbCol1 and ui.cbCol2;

Test Plan

Go to hypothesis Test Dock. Select Anova in test and in test_type select One way.
Perform Levene Test to check for homogeneity.
Click on Do Test to perform one way anova test.

Diff Detail

Repository
R262 LabPlot
Lint
Lint Skipped
Unit
Unit Tests Skipped
devanshuagarwal created this object with edit policy "Administrators".
Restricted Application added a project: KDE Edu. · View Herald TranscriptJun 21 2019, 8:44 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
devanshuagarwal requested review of this revision.Jun 21 2019, 8:44 PM
sgerlach added inline comments.Jun 22 2019, 5:17 PM
src/backend/hypothesis_test/HypothesisTest.cpp
471

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

476

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

489

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

506

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

544

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

978

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

994

not gsl_cdf_gaussian_P?

src/kdefrontend/dockwidgets/HypothesisTestDock.cpp
77

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

253

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

src/kdefrontend/dockwidgets/HypothesisTestDock.h
77

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)?

devanshuagarwal marked 5 inline comments as done.Jun 22 2019, 7:19 PM
devanshuagarwal added inline comments.
src/backend/hypothesis_test/HypothesisTest.cpp
476

This function clears variables in HypothesisTestView, so that new results can be added. We dont actually have that many global variables.

506

Thanks :)

978

Yes, firstly I used gsl_cdf_tdis_P(value, df) + gsl_cdf_tdis_P(-value, df). but the results from JASP and online calculator are not matching with this. The results are matching with 2*gsl_cdf_tdis_P(value, df)

994

yes it should be gsl_cdf_gaussian_P. Actually, I have left ztest backend for now, I am not able to find online calculator and neither ztest is included in JASP. So for time being, I am not concentrating on it.

src/kdefrontend/dockwidgets/HypothesisTestDock.cpp
253

Is this not correct or less efficient?

src/kdefrontend/dockwidgets/HypothesisTestDock.h
77

No, technically we dont need separate variables, but it avoids confusion and also gives us the clear overview.

I will think about using bit-field, but again, it will make things complex and will increase bugs.

sgerlach added inline comments.Jun 22 2019, 7:59 PM
src/backend/hypothesis_test/HypothesisTest.cpp
476

Can you rename the function to "clearTestView" or something like this to make the purpose clearer? Are the global vars necessary? If not, please avoid them.

978

OK. Please add a TODO-comment about this issue so we can check it later again.

994

Please add a TODO-comment in the code that the value is not tested.

src/kdefrontend/dockwidgets/HypothesisTestDock.cpp
253

It's surely less efficient since you call currentText() three times and compare each time to a string. Also the comparison will fail when the text is translated.

src/kdefrontend/dockwidgets/HypothesisTestDock.h
77

If it would be more complex, leave it like it is. I just wanted to point out other popular solutions.

devanshuagarwal marked 3 inline comments as done.Jun 24 2019, 11:35 AM
src/kdefrontend/dockwidgets/HypothesisTestDock.h
77

I too agree that the current setup is making code dirty. I think we should tinker on your idea of using bit-field. But, I am not getting the clear idea of how to proceed on it.

Will each separate test will correspond to one bit?

sgerlach accepted this revision.Jun 24 2019, 8:44 PM

All open problems are addressed in D22085.

This revision is now accepted and ready to land.Jun 24 2019, 8:44 PM

Can this be closed?

Can this be closed?

I have tried doing all the rectification suggested by you. I think this should be closed if no other changes need to be done.
The rectified code diff can be found in : https://phabricator.kde.org/D22428

sgerlach requested changes to this revision.Jul 14 2019, 9:00 AM

please close.

This revision now requires changes to proceed.Jul 14 2019, 9:00 AM
sgerlach accepted this revision.Jul 14 2019, 9:00 AM
This revision is now accepted and ready to land.Jul 14 2019, 9:00 AM
asemke accepted this revision.Jul 14 2019, 9:22 AM