Skip to content
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.

Sign up for GitHub

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

R Syntax: split analysis and analysisForm #5764

Merged
merged 5 commits into from
Dec 20, 2024
Merged

R Syntax: split analysis and analysisForm #5764

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6083ea2
Prepare R Syntac
boutinb Dec 20, 2024
3f99cfc
remove useless QJsonDocument
JorisGoosen Dec 20, 2024
b3db05d
Tabs
JorisGoosen Dec 20, 2024
51e6379
spacen
JorisGoosen Dec 20, 2024
b704152
Tabs
JorisGoosen Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CommonData/dataset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,3 +598,24 @@ stringset DataSet::findUsedColumnNames(std::string searchThis)

return columnsFound;
}

bool DataSet::initColumnWithStrings(int colIndex, const std::string & newName, const stringvec &values, const stringvec & labels, const std::string & title, columnType desiredType, const stringset & emptyValues, int threshold, bool orderLabelsByValue)
{
Column * column = columns()[colIndex];
column -> setHasCustomEmptyValues(emptyValues.size());
column -> setCustomEmptyValues(emptyValues);
column -> setName(newName);
column -> setTitle(title);
column -> beginBatchedLabelsDB();
bool anyChanges = title != column->title() || newName != column->name();
columnType prevType = column->type(),
suggestedType = column->setValues(values, labels, threshold, &anyChanges); //If less unique integers than the thresholdScale then we think it must be ordinal: https://github.com/jasp-stats/INTERNAL-jasp/issues/270
column -> setType(column->type() != columnType::unknown ? column->type() : desiredType == columnType::unknown ? suggestedType : desiredType);
column -> endBatchedLabelsDB();

if(orderLabelsByValue)
column->labelsOrderByValue();

return anyChanges || column->type() != prevType;
}

1 change: 1 addition & 0 deletions CommonData/dataset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class DataSet : public DataSetBaseNode

void loadOldComputedColumnsJson(const Json::Value & json); ///< Should act the same as the old ComputedColumns::fromJson() to allow loading "older jaspfiles"
stringset findUsedColumnNames(std::string searchThis);
bool initColumnWithStrings(int colIndex, const std::string & newName, const stringvec &values, const stringvec & labels, const std::string & title, columnType desiredType, const stringset & emptyValues, int threshold, bool orderLabelsByValue);

DatabaseInterface & db();
const DatabaseInterface & db() const;
Expand Down
22 changes: 4 additions & 18 deletions Desktop/data/datasetpackage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1488,24 +1488,10 @@ bool DataSetPackage::initColumnWithStrings(QVariant colId, const std::string & n
{
JASPTIMER_SCOPE(DataSetPackage::initColumnWithStrings);

int colIndex = getColIndex(colId),
threshold = Settings::value(Settings::THRESHOLD_SCALE).toInt();
Column * column = _dataSet->columns()[colIndex];
column -> setHasCustomEmptyValues(emptyValues.size());
column -> setCustomEmptyValues(emptyValues);
column -> setName(newName);
column -> setTitle(title);
column -> beginBatchedLabelsDB();
bool anyChanges = title != column->title() || newName != column->name();
columnType prevType = column->type(),
suggestedType = column->setValues(values, labels, threshold, &anyChanges); //If less unique integers than the thresholdScale then we think it must be ordinal: https://github.com/jasp-stats/INTERNAL-jasp/issues/270
column -> setType(column->type() != columnType::unknown ? column->type() : desiredType == columnType::unknown ? suggestedType : desiredType);
column -> endBatchedLabelsDB();

if(PreferencesModel::prefs()->orderByValueByDefault())
column->labelsOrderByValue();

return anyChanges || column->type() != prevType;
return _dataSet->initColumnWithStrings(
getColIndex(colId), newName, values, labels, title, desiredType, emptyValues,
Settings::value(Settings::THRESHOLD_SCALE).toInt(),
PreferencesModel::prefs()->orderByValueByDefault());
}

void DataSetPackage::initializeComputedColumns()
Expand Down
1 change: 0 additions & 1 deletion Engine/engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

#include "enginebase.h"
#include "enginedefinitions.h"
#include "dataset.h"
#include "ipcchannel.h"
#include <json/json.h>
#include "columnencoder.h"
Expand Down
56 changes: 33 additions & 23 deletions QMLComponents/analysisform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "knownissues.h"
#include "boundcontrols/boundcontrol.h"
#include "utilities/qutils.h"
#include "models/listmodeltermsavailable.h"
#include "controls/jasplistcontrol.h"
#include "controls/expanderbuttonbase.h"
#include "log.h"
Expand All @@ -31,7 +30,8 @@
#include <QQmlContext>
#include <QQmlEngine>
#include <QTimer>
#include "controls/variableslistbase.h"
#include <QJsonDocument>
#include <QJsonObject>
#include "preferencesmodelbase.h"

using namespace std;
Expand Down Expand Up @@ -110,7 +110,7 @@ void AnalysisForm::cleanUpForm()
}

void AnalysisForm::runScriptRequestDone(const QString& result, const QString& controlName, bool hasError)
{
{
if(_removed)
return;

Expand All @@ -135,11 +135,10 @@ void AnalysisForm::runScriptRequestDone(const QString& result, const QString& co
// Some controls generate extra controls (rowComponents): these extra controls must be first destroyed, because they may disturb the binding of other controls
// For this, bind all controls to null and wait for the controls to be completely destroyed.
QTimer::singleShot(0, [=](){
bindTo(options);
bindTo(options);
JorisGoosen marked this conversation as resolved.
Show resolved Hide resolved
blockValueChangeSignal(false, false);
_analysis->boundValueChangedHandler();
_analysis->boundValueChangedHandler();
JorisGoosen marked this conversation as resolved.
Show resolved Hide resolved
});

}
}

Expand Down Expand Up @@ -170,15 +169,14 @@ void AnalysisForm::filterByNameDone(const QString & name, const QString & error)
control->filterDoneHandler(name, error);
}


void AnalysisForm::addControl(JASPControl *control)
{
const QString & name = control->name();

if (_analysis && control->isBound())
{
connect(control, &JASPControl::requestColumnCreation, _analysis, &AnalysisBase::requestColumnCreationHandler);

connect(control, &JASPControl::usedVariablesChanged, _analysis, &AnalysisBase::onUsedVariablesChanged);
}

Expand Down Expand Up @@ -272,16 +270,28 @@ void AnalysisForm::setHasVolatileNotes(bool hasVolatileNotes)
emit hasVolatileNotesChanged();
}

bool AnalysisForm::parseOptions(Json::Value &options)

QString AnalysisForm::parseOptions(QString options)
{
if (_rSyntax->parseRSyntaxOptions(options))
Json::Reader jsonReader;
Json::Value jsonOptions;
Json::Value jsonResult(Json::objectValue);

QJsonDocument doc = QJsonDocument::fromJson(options.toUtf8());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this step here?
It seems that the whole QJsonDocument

jsonReader.parse(doc.toJson().toStdString(), jsonOptions, false);

if (!_analysis)
setAnalysis(new AnalysisBase(this)); // Create a dummy analyis object

if (_rSyntax->parseRSyntaxOptions(jsonOptions))
{
bindTo(options);
options = _analysis->boundValues();
return true;
bindTo(jsonOptions);
jsonOptions = _analysis->boundValues();
}

return false;
jsonResult["options"] = jsonOptions;
jsonResult["error"] = fq(getError());
return tq(jsonResult.toStyledString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return the Json value then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the jaspQMLR example, the QML form is just a QuickItem object, so I call the parseOptions with the QMetaObject::invokeMethod method:
https://github.com/jasp-stats/jaspQmlR/blob/a7a93a2f6fa103d47560c1ed541e9c252eed3671/src/qmlR.cpp#L208
In this way I don't need to load the AnalysisFormBase header in the jaspQMLR package.
I don't think I can pass a Json::Value argument in the InvokeMethod, so I thought just send the string, and reset it as Json object afterwards.

}

void AnalysisForm::_setUp()
Expand Down Expand Up @@ -311,7 +321,7 @@ void AnalysisForm::reset()

void AnalysisForm::exportResults()
{
_analysis->exportResults();
_analysis->exportResults();
}

QString AnalysisForm::msgsListToString(const QStringList & list) const
Expand Down Expand Up @@ -370,7 +380,7 @@ void AnalysisForm::_addLoadingError(QStringList wrongJson)
void AnalysisForm::bindTo(const Json::Value & defaultOptions)
{
std::set<std::string> controlsJsonWrong;

for (JASPControl* control : _dependsOrderedCtrls)
{
BoundControl* boundControl = control->boundControl();
Expand Down Expand Up @@ -567,7 +577,7 @@ void AnalysisForm::setTitle(QString title)
_analysis->setTitle(fq(title.simplified()));
}

void AnalysisForm::setOptionNameConversion(const QVariantList & conv)
void AnalysisForm::setOptionNameConversion(const QVariantList &conv)
JorisGoosen marked this conversation as resolved.
Show resolved Hide resolved
{
if (_rSyntax->setControlNameToRSyntaxMap(conv))
emit optionNameConversionChanged();
Expand Down Expand Up @@ -691,7 +701,7 @@ void AnalysisForm::blockValueChangeSignal(bool block, bool notifyOnceUnblocked)
else
{
_valueChangedSignalsBlocked--;

if (_valueChangedSignalsBlocked < 0)
_valueChangedSignalsBlocked = 0;

Expand All @@ -701,7 +711,7 @@ void AnalysisForm::blockValueChangeSignal(bool block, bool notifyOnceUnblocked)
_analysis->boundValueChangedHandler();

_valueChangedEmittedButBlocked = false;

if(_analysis && (notifyOnceUnblocked || _analysis->wasUpgraded())) //Maybe something was upgraded and we want to run the dropped rscripts (for instance for https://github.com/jasp-stats/INTERNAL-jasp/issues/1399)
{
while(_waitingRScripts.size() > 0)
Expand All @@ -713,7 +723,7 @@ void AnalysisForm::blockValueChangeSignal(bool block, bool notifyOnceUnblocked)

for(const auto & filterName : _waitingFilters)
_analysis->sendFilter(filterName);

_waitingFilters.clear();
}
else //Otherwise just clean it up
Expand Down Expand Up @@ -912,14 +922,14 @@ QString AnalysisForm::helpMD() const
markdown << control->helpMD() << "\n";

markdown << metaHelpMD();

if(!_infoBottom.isEmpty())
markdown << "\n\n---\n" << _infoBottom << "\n";

QString md = markdown.join("");

_analysis->preprocessMarkdownHelp(md);

return md;
}

Expand Down
6 changes: 3 additions & 3 deletions QMLComponents/analysisform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ public slots:
Q_INVOKABLE void refreshAnalysis();
Q_INVOKABLE bool initialized() const { return _initialized; }
Q_INVOKABLE QString generateWrapper() const;
Q_INVOKABLE QString parseOptions(QString options);
JorisGoosen marked this conversation as resolved.
Show resolved Hide resolved

void addControlError(JASPControl* control, QString message, bool temporary = false, bool warning = false, bool closeable = true);
void clearControlError(JASPControl* control);
Expand Down Expand Up @@ -180,9 +181,8 @@ public slots:
void sortControls(QList<JASPControl*>& controls);
QString getSyntaxName(const QString& name) const;
void setHasVolatileNotes(bool hasVolatileNotes);
bool parseOptions(Json::Value& options);
void setActiveJASPControl(JASPControl* control, bool hasActiveFocus);
JASPControl* getActiveJASPControl() { return _activeJASPControl; }
void setActiveJASPControl(JASPControl* control, bool hasActiveFocus);
JASPControl* getActiveJASPControl() { return _activeJASPControl; }
JorisGoosen marked this conversation as resolved.
Show resolved Hide resolved

static const QString rSyntaxControlName;

Expand Down
Loading