D21496: Two Sample T-test Backend

[Stefan Gerlach]

sgerlach added a comment.


  first comments.

INLINE COMMENTS

> CMakeLists.txt:239
>  	${BACKEND_DIR}/pivot/PivotTable.cpp
> +        ${BACKEND_DIR}/t_test/ttest.cpp
>  	${BACKEND_DIR}/spreadsheet/Spreadsheet.cpp

do we need a directory for every test?
TTest.cpp instead of ttest.cpp would be more standard.

> ttest.cpp:14
> +TTest::TTest(const QString &name)
> +{
> +    Q_UNUSED(name);

please put the '{' after the function  definition. This saves some space and is our code style :-)

> ttest.cpp:24
> +    m_columns = dataSourceSpreadsheet->columnCount();
> +    qDebug() << "in ttest::setDataSourceSpreadsheet";
> +

We have a macro "DEBUG" to generalize debugging. Please use this (and remove "#include <QDEBUG>")

> ttest.cpp:44
> +    {
> +        msg_box->setText(QLatin1String("Inappropriate number of columns selected"));
> +        msg_box->exec();

all user visible text should be translatable (i18n())

> ttest.cpp:50
> +    bool modeOk = true;
> +    for(int i=0; i<2; i++)
> +    {

use spaces: for (int i = 0; i < 2; i++)

> ttest.cpp:59
> +    {
> +        msg_box->setText("select only columns with numbers");
> +        msg_box->exec();

translation (s.a.)

> ttest.cpp:91
> +    double t = (mean[1] - mean[2])/(sp*qSqrt(1.0/n[1] + 1.0/n[2]));
> +    QString text = QString("T value for test is %1").arg(t);
> +    msg_box->setText(text);

translation (s.a.)

> SpreadsheetView.cpp:225
>  	action_pivot_table = new QAction(QIcon::fromTheme("table"), i18n("Pivot Table"), this);
> +    action_do_ttest = new QAction(i18n("T Test"),this);
>  

space after ','

> SpreadsheetView.h:196
>  	QMenu* m_analyzePlotMenu;
> +    QMenu* m_columnHypotheisTestingMenu{nullptr};
>  

"Hypothe_s_is"

REPOSITORY
  R262 LabPlot

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

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