Skip to content

Commit

Permalink
More layout updates:
Browse files Browse the repository at this point in the history 
* Parse strings with less copy/paste of bison code.
* Fix error typo.
* Disallow negative sizes.
* Turn on layout with fewer cues.
* Add layout/render tests.
  • Loading branch information
@luciansmith
luciansmith committed Oct 1, 2024
1 parent 2492862 commit 1364e37
Show file tree
Hide file tree
Showing 6 changed files with 1,401 additions and 1,292 deletions.
2,468 changes: 1,219 additions & 1,249 deletions src/antimony.tab.cpp
View file

Large diffs are not rendered by default.

47 changes: 9 additions & 38 deletions src/antimony.ypp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ using namespace libsbml;
%type <stringlist> stringlist
%type <variablelist> variablelist
%type <numlist> numlist
%type <word> stringConstant

%left '&' '|' /* Boolean functions and, or */
%left '-' '+'
Expand Down Expand Up @@ -286,17 +287,7 @@ moduleannotation: MODULE ANTWORD stringlist lineend
YYABORT;
}
}
| MODULE '.' ANTWORD '=' ANTWORD
{ Module* module = g_registry.CurrentModule();
if (module && $3 && CaselessStrCmp(true, *($3), "layout")) {
if (module->SetLayout($5)) YYABORT;
}
else {
g_registry.SetError("Invalid syntax 'model." + *($3) + "'. The only thing that can be set on the model to a string is 'layout'.");
YYABORT;
}
}
| MODULE '.' ANTWORD '=' CONSTANT
| MODULE '.' ANTWORD '=' stringConstant
{ Module* module = g_registry.CurrentModule();
if (module && $3 && CaselessStrCmp(true, *($3), "layout")) {
if (module->SetLayout($5)) YYABORT;
Expand All @@ -320,21 +311,7 @@ moduleannotation: MODULE ANTWORD stringlist lineend
}
module->SetLayout("on");
}
| MODULE '.' ANTWORD '.' ANTWORD '=' ANTWORD
{ Module* module = g_registry.CurrentModule();
if (module && $3 && CaselessStrCmp(true, *($3), "autolayout")) {
if (module->SetAutoLayout($5, $7)) YYABORT;
}
else if (module && $3 && CaselessStrCmp(true, *($3), "layout")) {
if (module->SetLayout($5, $7)) YYABORT;
}
else {
g_registry.SetError("Invalid syntax 'model." + *($3) + "'.*: The core word here must be 'layout' or 'autolayout'.");
YYABORT;
}
module->SetLayout("on");
}
| MODULE '.' ANTWORD '.' ANTWORD '=' TEXTSTRING
| MODULE '.' ANTWORD '.' ANTWORD '=' stringConstant
{ Module* module = g_registry.CurrentModule();
if (module && $3 && CaselessStrCmp(true, *($3), "autolayout")) {
if (module->SetAutoLayout($5, $7)) YYABORT;
Expand Down Expand Up @@ -378,6 +355,10 @@ moduleannotation: MODULE ANTWORD stringlist lineend
}
;

stringConstant: CONSTANT {$$ = $1;}
| TEXTSTRING {$$ = $1;}
| ANTWORD {$$ = $1;}

variablelist: variable { $$ = new std::vector<Variable*>(); $$->push_back($1);} // need to free $1?
| variablelist ',' variable { $$ = $1; $$->push_back($3); } // need to free $3?
| variablelist ',' '\n' variable { $$ = $1; $$->push_back($4); } // eat newlines
Expand Down Expand Up @@ -732,17 +713,7 @@ toplevel_sbo: MODNAME '.' ANTWORD '=' NUM
YYABORT;
}
}
| MODNAME '.' ANTWORD '=' ANTWORD
{ Module* function = g_registry.GetModule(*$1);
if (function && $3 && CaselessStrCmp(true, *($3), "autolayout")) {
if (function->SetLayout($5)) YYABORT;
}
else {
g_registry.SetError("Invalid syntax '" + *($1) + "." + *($3) + "'. The only thing that can set on '" + *($1) + "' to a string is 'layout'.");
YYABORT;
}
}
| MODNAME '.' ANTWORD '=' CONSTANT
| MODNAME '.' ANTWORD '=' stringConstant
{ Module* function = g_registry.GetModule(*$1);
if (function && $3 && CaselessStrCmp(true, *($3), "autolayout")) {
if (function->SetLayout($5)) YYABORT;
Expand All @@ -766,7 +737,7 @@ toplevel_sbo: MODNAME '.' ANTWORD '=' NUM
}
module->SetLayout("on");
}
| MODNAME '.' ANTWORD '.' ANTWORD '=' ANTWORD
| MODNAME '.' ANTWORD '.' ANTWORD '=' stringConstant
{ Module* module = g_registry.CurrentModule();
if (module && $3 && CaselessStrCmp(true, *($3), "autolayout")) {
if (module->SetAutoLayout($5, $7)) YYABORT;
Expand Down
14 changes: 9 additions & 5 deletions src/layoutWrapper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ bool LayoutWrapper::SetFormula(Formula* formula, bool isObjective)
#ifndef NSBML
ASTNode* astn = parseStringToASTNode(formula->ToSBMLString());
if (!astn || astn->getType() != AST_LINEAR_ALGEBRA_VECTOR || astn->getNumChildren() != 2) {
g_registry.SetError("Unable to set the value of '" + GetNameDelimitedBy(".") + "' to be '" + formula->ToDelimitedStringWithEllipses(".") + "': an layout parameter of type " + LayoutTypeToString(m_layout_type) + " must be a vector of length two, marked with curly brackets (i.e. '{150, 200}').");
g_registry.SetError("Unable to set the value of '" + GetNameDelimitedBy(".") + "' to be '" + formula->ToDelimitedStringWithEllipses(".") + "': a layout parameter of type " + LayoutTypeToString(m_layout_type) + " must be a vector of length two, marked with curly brackets (i.e. '{150, 200}').");
delete astn;
return true;
}
for (unsigned int c = 0; c < 2; c++) {
ASTNodeType_t ctype = astn->getChild(c)->getType();
if (!astn->getChild(c)->isNumber()) {
g_registry.SetError("Unable to set the value of '" + GetNameDelimitedBy(".") + "' to be '" + formula->ToDelimitedStringWithEllipses(".") + "': an layout parameter of type " + LayoutTypeToString(m_layout_type) + " must be a vector of length two, and each element of the vector may only be a value (i.e. '{150, 200}').");
g_registry.SetError("Unable to set the value of '" + GetNameDelimitedBy(".") + "' to be '" + formula->ToDelimitedStringWithEllipses(".") + "': a layout parameter of type " + LayoutTypeToString(m_layout_type) + " must be a vector of length two, and each element of the vector may only be a value (i.e. '{150, 200}').");
delete astn;
return true;
}
Expand All @@ -114,8 +114,12 @@ bool LayoutWrapper::SetFormula(Formula* formula, bool isObjective)
delete astn;
return true;
}
if (m_layout_type == lt_size && (astn->getChild(0)->getValue() < 0 || astn->getChild(1)->getValue() < 0)) {
g_registry.SetError("Unable to set the value of '" + GetNameDelimitedBy(".") + "' to be '" + formula->ToDelimitedStringWithEllipses(".") + "': size may not be negative.");
delete astn;
return true;
}
delete astn;
#endif
return false;
}
else {
Expand Down Expand Up @@ -189,6 +193,7 @@ bool LayoutWrapper::SetFormula(Formula* formula, bool isObjective)
g_registry.SetError("Unable to set the value of '" + GetNameDelimitedBy(".") + "' to be '" + formula->ToDelimitedStringWithEllipses(".") + "': this layout parameter must only be a single value or a single variable.");
return true;
}
#endif
return false;
}

Expand Down Expand Up @@ -228,7 +233,7 @@ string LayoutWrapper::GetNameDelimitedBy(string cc) const

bool LayoutWrapper::Synchronize(Variable* clone, const Variable* conversionFactor)
{
g_registry.SetError("Unable to synchronize two symbols when one of them ('" + GetNameDelimitedBy(".") + "') is an layout term.");
g_registry.SetError("Unable to synchronize two symbols when one of them ('" + GetNameDelimitedBy(".") + "') is a layout term.");
return true;
}

Expand Down Expand Up @@ -386,7 +391,6 @@ bool LayoutWrapper::TransferLayoutInformationTo(SBMLDocument* sbml, const string
assert(m_layout_type == lt_size);
if (group == "species") {
ret1 = LIBSBMLNETWORK_CPP_NAMESPACE::setSpeciesDimensionWidth(sbml, 0, xval);
//double S1width = LIBSBMLNETWORK_CPP_NAMESPACE::getDimensionWidth(sbml, "S1");
ret2 = LIBSBMLNETWORK_CPP_NAMESPACE::setSpeciesDimensionHeight(sbml, 0, yval);
}
else if (group == "compartment") {
Expand Down
2 changes: 2 additions & 0 deletions src/module.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3202,6 +3202,7 @@ bool Module::SetAutoLayout(const std::string* argument, const std::string* value
}
else if (CaselessStrCmp(true, *argument, "useNameAsTextLabel")) {
m_autolayout.useNameAsTextLabel = arg;
m_autolayout.use = true;
}
return false;
}
Expand Down Expand Up @@ -3238,6 +3239,7 @@ bool Module::SetAutoLayout(const string* argument, const double& value)
//}
else if (CaselessStrCmp(true, *argument, "maxNumConnectedEdges")) {
m_autolayout.maxNumConnectedEdges = int(round(value));
m_autolayout.use = true;
}
return false;
}
Expand Down
159 changes: 159 additions & 0 deletions src/test/TestAntimonyErrors.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,138 @@ START_TEST (uncertval_must_be_single2)
}
END_TEST

START_TEST(no_aligning_set_species)
{
testError("species S1, S2, S3; S1.x = 3; S2.x = 5; model.layout.align_top = {S1, S2}", "Error in model string, line 1: Unable to set the alignment align_top because only one element is allowed in an alignment whose position is already known. In addition, adding an element to an alignment then establishes that element's position, so two such elements cannot be used in another alignment. In this case, the positions of elements 'S1', and 'S2' are already known.");
}
END_TEST

START_TEST(no_aligning_already_aligned)
{
testError("species S1, S2, S3; model.layout.align_bottom = {S1, S2}; model.layout.align_top = {S1, S2}", "Error in model string, line 1: Unable to set the alignment align_bottom because only one element is allowed in an alignment whose position is already known. In addition, adding an element to an alignment then establishes that element's position, so two such elements cannot be used in another alignment. In this case, the positions of elements 'S1', and 'S2' are already known.");
}
END_TEST

START_TEST(no_aligning_alignment_set_mix)
{
testError("species S1, S2, S3; S3.x = 5; model.layout.align_bottom = {S3, S2}; model.layout.align_top = {S1, S2}", "Error in model string, line 1: Unable to set the alignment align_bottom because only one element is allowed in an alignment whose position is already known. In addition, adding an element to an alignment then establishes that element's position, so two such elements cannot be used in another alignment. In this case, the positions of elements 'S2', and 'S3' are already known.");
}
END_TEST

START_TEST(no_aligning_alignment_set_mix_more_species)
{
testError("species S1, S2, S3, S4, S5, S6; S3.x = 5; model.layout.align_top = {S1, S2}; model.layout.align_bottom = {S1, S4, S6, S3}; ", "Error in model string, line 1: Unable to set the alignment align_bottom because only one element is allowed in an alignment whose position is already known. In addition, adding an element to an alignment then establishes that element's position, so two such elements cannot be used in another alignment. In this case, the positions of elements 'S1', and 'S3' are already known.");
}
END_TEST

START_TEST(no_layout_for_events)
{
testError("E0: at time>3:x=4; E0.color = red", "Error in model string, line 1: Unable to add layout or render information to E0: only species, reactions, and compartments can be visualized, and this element is of type 'Event'.");
}
END_TEST

START_TEST(pos_must_be_vector)
{
testError("species S1;S1.pos = 5", "Error in model string, line 1: Unable to set the value of 'S1.position' to be '5': a layout parameter of type position must be a vector of length two, marked with curly brackets (i.e. '{150, 200}').");
}
END_TEST

START_TEST(size_must_be_vector)
{
testError("species S1;S1.size = 5", "Error in model string, line 1: Unable to set the value of 'S1.size' to be '5': a layout parameter of type size must be a vector of length two, marked with curly brackets (i.e. '{150, 200}').");
}
END_TEST

START_TEST(pos_must_be_len2_vectorA)
{
testError("species S1;S1.pos = {5}", "Error in model string, line 1: Unable to set the value of 'S1.position' to be '{5}': a layout parameter of type position must be a vector of length two, marked with curly brackets (i.e. '{150, 200}').");
}
END_TEST

START_TEST(pos_must_be_len2_vectorB)
{
testError("species S1;S1.pos = {5, 4, 4}", "Error in model string, line 1: Unable to set the value of 'S1.position' to be '{5, 4, 4}': a layout parameter of type position must be a vector of length two, marked with curly brackets (i.e. '{150, 200}').");
}
END_TEST

START_TEST(no_unit_in_position)
{
testError("species S1;S1.pos = {5 cm, 4 mm}", "Error in model string, line 1: Unable to set the value of 'S1.position' to be '{5 cm, 4 mm}': units may not be used for layout positions.");
}
END_TEST

START_TEST(only_nums_in_position)
{
testError("species S1;S1.pos = {5, red}", "Error in model string, line 1: Unable to set the value of 'S1.position' to be '{5, red}': a layout parameter of type position must be a vector of length two, and each element of the vector may only be a value (i.e. '{150, 200}').");
}
END_TEST

START_TEST(x_only_number)
{
testError("species S1;S1.x = red", "Error in model string, line 1: Unable to set the value of 'S1.x' to 'red'. It may only be set to a numerical value.");
}
END_TEST

START_TEST(color_not_number)
{
testError("species S1;S1.color = 5", "Error in model string, line 1: Unable to set the value of 'S1.color'. '5' is not a valid color value. Try standard color names like 'red' or 'blue', or use an RGB value of the form \"#000000\" (including the quotation marks).");
}
END_TEST

START_TEST(color_not_variable)
{
testError("species S1;S1.color = S2", "Error in model string, line 1: Unable to set the value of 'S1.color'. 'S2' is not a valid color value. Try standard color names like 'red' or 'blue', or use an RGB value of the form \"#000000\" (including the quotation marks).");
}
END_TEST

START_TEST(fontstyle_not_number)
{
testError("species S1;S1.fontstyle = 43", "Error in model string, line 1: Unable to set the value of 'S1.fontStyle'. '43' is not a valid font style. The valid font styles and weights are 'normal', 'bold', 'italic', and 'bold_italic'.");
}
END_TEST

START_TEST(fontstyle_not_variable)
{
testError("species S1;S1.fontstyle = S2", "Error in model string, line 1: Unable to set the value of 'S1.fontStyle'. 'S2' is not a valid font style. The valid font styles and weights are 'normal', 'bold', 'italic', and 'bold_italic'.");
}
END_TEST

START_TEST(shape_not_number)
{
testError("species S1;S1.shape = 43", "Error in model string, line 1: Unable to set the value of 'S1.shape'. '43' is not a valid shape name. The valid shape names are 'rectangle', 'square', 'ellipse', 'circle', 'triangle', 'diamond', 'pentagon', 'hexagon', and 'octagon'.");
}
END_TEST

START_TEST(x_not_vector)
{
testError("species S1;S1.x = {43, 44}", "Error in model string, line 1: Unable to set the value of 'S1.x' to be '{43, 44}': this layout parameter must only be a single value or a single variable.");
}
END_TEST

START_TEST(layout_not_reactant)
{
testError("S1.x->;", "Error in model string, line 1: The variable 'S1.x' cannot be used in a reaction or interaction, as it is the wrong type ('Layout or render parameter').");
}
END_TEST

START_TEST(layout_not_species)
{
testError("species S1.x", "Error in model string, line 1: Unable to use the symbol 'S1.x' in any context other than setting its value.");
}
END_TEST

START_TEST(no_sync_layout_stuff)
{
testError("species S1, S2; S2 is S1.x", "Error in model string, line 1: Unable to synchronize two symbols when one of them ('S1.x') is a layout term.");
}
END_TEST

START_TEST(no_negative_size)
{
testError("s1->; ;species.size = {-5, 5}", "Error in model string, line 1: Unable to set the value of 'species.size' to be '{ - 5, 5}': size may not be negative.");
}
END_TEST

/*
START_TEST (no_replace_ar_with_ia)
{
Expand All @@ -373,6 +505,13 @@ create_suite_Errors (void)
Suite *suite = suite_create("Antimony Errors");
TCase *tcase = tcase_create("Antimony Errors");

tcase_add_test(tcase, layout_not_species);
tcase_add_test(tcase, no_sync_layout_stuff);
tcase_add_test(tcase, no_negative_size);
tcase_add_test(tcase, unknown_file1);
tcase_add_test(tcase, unknown_file1);
tcase_add_test(tcase, unknown_file1);

tcase_add_test( tcase, unknown_file1);
tcase_add_test( tcase, unknown_file2);
tcase_add_test( tcase, unknown_file3);
Expand Down Expand Up @@ -414,6 +553,26 @@ create_suite_Errors (void)
tcase_add_test( tcase, uncertval_must_be_single1);
tcase_add_test( tcase, uncertval_must_be_single2);

tcase_add_test(tcase, no_aligning_set_species);
tcase_add_test(tcase, no_aligning_already_aligned);
tcase_add_test(tcase, no_aligning_alignment_set_mix);
tcase_add_test(tcase, no_aligning_alignment_set_mix_more_species);
tcase_add_test(tcase, no_layout_for_events);
tcase_add_test(tcase, pos_must_be_vector);
tcase_add_test(tcase, size_must_be_vector);
tcase_add_test(tcase, pos_must_be_len2_vectorA);
tcase_add_test(tcase, pos_must_be_len2_vectorB);
tcase_add_test(tcase, no_unit_in_position);
tcase_add_test(tcase, only_nums_in_position);
tcase_add_test(tcase, x_only_number);
tcase_add_test(tcase, color_not_number);
tcase_add_test(tcase, color_not_variable);
tcase_add_test(tcase, fontstyle_not_number);
tcase_add_test(tcase, fontstyle_not_variable);
tcase_add_test(tcase, shape_not_number);
tcase_add_test(tcase, x_not_vector);
tcase_add_test(tcase, layout_not_reactant);

suite_add_tcase(suite, tcase);

return suite;
Expand Down
3 changes: 3 additions & 0 deletions src/test/TestRunner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Suite *create_suite_Hierarchy(void);
Suite *create_suite_Flattening(void);
Suite *create_suite_Distrib(void);
Suite *create_suite_FBC(void);
Suite* create_suite_LayoutRender(void);
Suite *create_suite_Constraints(void);
Suite *create_suite_CVTerms(void);
Suite *create_suite_SBO(void);
Expand Down Expand Up @@ -131,6 +132,7 @@ main (int argc, char* argv[])
//SRunner *runner = srunner_create( create_suite_Flattening() );
//SRunner *runner = srunner_create( create_suite_Distrib() );
//SRunner *runner = srunner_create( create_suite_FBC() );
//SRunner *runner = srunner_create( create_suite_LayoutRender() );
//SRunner *runner = srunner_create( create_suite_Constraints() );
//SRunner* runner = srunner_create(create_suite_CVTerms());
//SRunner* runner = srunner_create(create_suite_SBO());
Expand All @@ -145,6 +147,7 @@ main (int argc, char* argv[])
srunner_add_suite( runner, create_suite_SBO());
srunner_add_suite( runner, create_suite_Distrib() );
srunner_add_suite( runner, create_suite_FBC() );
srunner_add_suite( runner, create_suite_LayoutRender() );
srunner_add_suite( runner, create_suite_Constraints() );
srunner_add_suite( runner, create_suite_CVTerms() );
srunner_add_suite( runner, create_suite_Uncert() );
Expand Down

0 comments on commit 1364e37

Please sign in to comment.