Skip to content

Commit

Permalink
Order of variables should be kept in formula
Browse files Browse the repository at this point in the history
Fixes jasp-stats/INTERNAL-jasp#2689

Also in the Mixed Models, if several random effects are added, some effects are sometimes suddenly added or removed when applying the R Syntax. This was due to the fact that the controls dynamically created by the ComponentsList were not removed before setting the new options.
At last, avoid generating R Syntax during the initialization of the form: each time a control gets a new value, the R Syntax is generated: this is not needed when the form is being initialized.
  • Loading branch information
boutinb authored and JorisGoosen committed Nov 14, 2024
1 parent 5c1c694 commit d4810ee
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 19 deletions.
3 changes: 2 additions & 1 deletion QMLComponents/analysisbase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ void AnalysisBase::setBoundValue(const std::string &name, const Json::Value &val
else
Log::log() << "Could not find parent keys " << _displayParentKeys(parentKeys) << " in options: " << _boundValues.toStyledString() << std::endl;

emit boundValuesChanged();
if (_analysisForm->initialized())
emit boundValuesChanged();
}

void AnalysisBase::setBoundValues(const Json::Value &boundValues)
Expand Down
12 changes: 8 additions & 4 deletions QMLComponents/analysisform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,14 @@ void AnalysisForm::runScriptRequestDone(const QString& result, const QString& co
blockValueChangeSignal(true);
_analysis->clearOptions();
bindTo(Json::nullValue);
bindTo(options);
blockValueChangeSignal(false, false);
_analysis->boundValueChangedHandler();
// 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);
blockValueChangeSignal(false, false);
_analysis->boundValueChangedHandler();
});

}
}

Expand Down Expand Up @@ -598,7 +603,6 @@ void AnalysisForm::setAnalysisUp()
// Don't bind boundValuesChanged before it is initialized: each setup of all controls will generate a boundValuesChanged
connect(_analysis, &AnalysisBase::boundValuesChanged, this, &AnalysisForm::setRSyntaxText, Qt::QueuedConnection );

setRSyntaxText();
emit analysisChanged();
}

Expand Down
42 changes: 28 additions & 14 deletions QMLComponents/rsyntax/formulasource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,24 +257,25 @@ QString FormulaSource::_generateRandomEffectsTerms(const Terms& terms) const
return result;
}

QString FormulaSource::generateInteractionTerms(const Terms& tterms, const Json::Value& changedTypes)
QString FormulaSource::generateInteractionTerms(const Terms& terms, const Json::Value& changedTypes)
{
// If the terms has interactions, try to use the '*' symbol when all combinations of the subterms are also present in the terms.
QString result;
bool first = true;
std::vector<Term> terms = tterms.terms();
std::sort(terms.begin(), terms.end(), [](const Term& a, const Term& b){ return a.components().length() < b.components().length(); });
std::vector<Term> orgTerms = terms;
std::vector<Term> sortedTerms = terms.terms();
QMap<Term, Json::Value> termsToAdd;
std::sort(sortedTerms.begin(), sortedTerms.end(), [](const Term& a, const Term& b){ return a.components().length() < b.components().length(); });
std::vector<Term> orgTerms = sortedTerms;

while (!terms.empty())
// First try to find out whether if several terms can be reduced in a crass combination term: "a * b" means "a + b + a:b"
while (!sortedTerms.empty())
{
if (!first) result += " + ";
first = false;
const Term& term = terms.at(terms.size() - 1);
int orgInd = tterms.indexOf(term);
const Term& term = sortedTerms.at(sortedTerms.size() - 1);
int orgInd = terms.indexOf(term);
const Json::Value& changedType = changedTypes.size() > orgInd ? changedTypes[orgInd] : Json::nullValue;
terms.pop_back();
if (term.components().size() == 1) result += FormulaParser::transformToFormulaTerm(term, changedType);
sortedTerms.pop_back();
if (term.components().size() == 1)
termsToAdd[term] = changedType;
else
{
bool allComponentsAreAlsoInTerms = true;
Expand All @@ -294,14 +295,27 @@ QString FormulaSource::generateInteractionTerms(const Terms& tterms, const Json:
// Remove the components and their combinations in terms so that they don't appear in the formula
for (const Term& oneTerm : allCrossedTerms)
{
auto found = std::find(terms.begin(), terms.end(), oneTerm);
if (found != terms.end()) terms.erase(found);
auto found = std::find(sortedTerms.begin(), sortedTerms.end(), oneTerm);
if (found != sortedTerms.end()) sortedTerms.erase(found);
}

if (!first) result += " + ";
result += FormulaParser::transformToFormulaTerm(term, changedType, FormulaParser::allInterationsSeparator);
first = false;
}
else
result += FormulaParser::transformToFormulaTerm(term, changedType, FormulaParser::interactionSeparator);
termsToAdd[term] = changedType;
}
}

// Use the original order of the terms to add them in the formula
for (const Term term : terms)
{
if (termsToAdd.contains(term))
{
if (!first) result += " + ";
result += FormulaParser::transformToFormulaTerm(term, termsToAdd[term], FormulaParser::interactionSeparator);
first = false;
}
}

Expand Down

0 comments on commit d4810ee

Please sign in to comment.