D21977: One Way Anova Test
noreply at phabricator.kde.org
Sat Jun 22 20:59:58 BST 2019
sgerlach added inline comments.
> devanshuagarwal wrote in HypothesisTest.cpp:476
> This function clears variables in HypothesisTestView, so that new results can be added. We dont actually have that many global variables.
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.
> devanshuagarwal wrote in HypothesisTest.cpp: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)
OK. Please add a TODO-comment about this issue so we can check it later again.
> devanshuagarwal wrote in HypothesisTest.cpp: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.
Please add a TODO-comment in the code that the value is not tested.
> devanshuagarwal wrote in HypothesisTestDock.cpp:253
> Is this not correct or less efficient?
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.
> devanshuagarwal wrote in 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.
If it would be more complex, leave it like it is. I just wanted to point out other popular solutions.
To: devanshuagarwal, sgerlach, asemke
Cc: kde-edu, #labplot, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the kde-edu