Skip to content

Order of variables should be kept in formula #5733

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.

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

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

lazy solution :p

first = false;
}
}

Expand Down
Loading