Skip to content

Fixes invalid specification of significantDigits on a String with integer argument. - #4796

Open
modlfo wants to merge 1 commit into
modelica:masterfrom
modlfo:master
Open

Fixes invalid specification of significantDigits on a String with integer argument.#4796
modlfo wants to merge 1 commit into
modelica:masterfrom
modlfo:master

Conversation

@modlfo

@modlfo modlfo commented Jul 30, 2026

Copy link
Copy Markdown

I found a few cases inside DynamicSelect where the String() is called with an integer argument and the significantDigits are provided. That's invalid according to the specification.

The standard type coercion described in section 10.6.13 shall not be applied for the first argument of String. Hence, specifying significantDigits is an error when the first argument of String is an Integer expression.

@CLAassistant

CLAassistant commented Jul 30, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@maltelenz
maltelenz requested a review from AHaumer September 1, 2026 12:04
@HansOlsson

Copy link
Copy Markdown
Contributor

Note that this restriction was added/clarified in modelica/ModelicaSpecification#3262 (comment)

@MatthiasBSchaefer

Copy link
Copy Markdown
Contributor

What do you think about converting the integer into a real (by multiply with 1.0 ) and set significantDigits=3 ?
String( y*1.0, minimumLength=1, significantDigits=3)

Such, large integers are displayed in scientific notation
y=123456789 ==> "1.23e8"

@HansOlsson

Copy link
Copy Markdown
Contributor

What do you think about converting the integer into a real (by multiply with 1.0 ) and set significantDigits=3 ? String( y*1.0, minimumLength=1, significantDigits=3)

Such, large integers are displayed in scientific notation y=123456789 ==> "1.23e8"

Not sure. I don't know how common such large integers are for these blocks, and what people want to see for such values.

However, it also impacts smaller numbers so 1234 will be written as "1.23e+03". That uses more space, and seems less readable if you work with integers than plain "1234", but I don't know how relevant that is either.

@HansOlsson HansOlsson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems ok.

@HansOlsson

Copy link
Copy Markdown
Contributor

Looking closer I realized that the removed setting is significantDigits=0. That seems bad even for two-digit integers, so I think the change is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants