Skip to content

string.format extension should output scientific notation consistent with CEL spec #1100

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

Closed
1 of 4 tasks
zeitgeist87 opened this issue Jan 8, 2025 · 7 comments
Closed
1 of 4 tasks

Comments

@zeitgeist87
Copy link
Contributor

Describe the bug
The string.format extension currently outputs scientific notation rendered using a unicode multiplication sign × and superscript for the exponent 2.718280 × 10⁰⁰.

To Reproduce
Check which components this affects:

  • parser
  • checker
  • interpreter
  • string.format extension

Sample expression and input that reproduces the issue:

"%e".format([2.71828])

Test setup:
Existing test case.

Expected behavior
The output should instead be conforming to the CEL spec: 2.718280E+00 or 2.718280e+00

FLOAT_LIT      ::= -? DIGIT* . DIGIT+ EXPONENT? | -? DIGIT+ EXPONENT
DIGIT          ::= [0-9]
HEXDIGIT       ::= [0-9abcdefABCDEF]
EXPONENT       ::= [eE] [+-]? DIGIT+
@TristonianJones
Copy link
Collaborator

It turns out there were two issues:

It appears that both issues will be addressed, but the scope of this bug will be to temporarily strip narrow spaces on exponents until Golang updates to support the same.

@liggitt
Copy link
Contributor

liggitt commented Mar 19, 2025

The output should instead be conforming to the CEL spec: 2.718280E+00 or 2.718280e+00

Switching away from the multiplication sign and exponent would be a bigger change than just stripping the narrow spaces... it's not clear to me that the format library must output something that CEL could parse back as a number in the CEL grammar.

@liggitt
Copy link
Contributor

liggitt commented Mar 19, 2025

golang/text@221d88c now strips the spaces in the latest tag

@TristonianJones
Copy link
Collaborator

@liggitt the PR from @jcking versions the format change considering it could break tests.

In looking at the complexity introduced by locales, we opted to stick with C-format and remove locales since the outputs would likely only need to be developer-readable. At some point we may introduce locales, but we won't mix developer and non-developer formatting, e.g map / list format alongside locale numeric formatting.

@liggitt
Copy link
Contributor

liggitt commented Mar 19, 2025

@liggitt the PR from @jcking versions the format change considering it could break tests.

as the golang.org/x/text change did when we bumped things in kubernetes - kubernetes/kubernetes#130913 (comment)

will CEL still inherit the narrow space stripping change from x/text?

@TristonianJones
Copy link
Collaborator

Yes, unfortunately, it will. Do you need us to undo it in a patch?

@liggitt
Copy link
Contributor

liggitt commented Mar 19, 2025

Not necessarily, just trying to figure out how we'll react downstream in a way we won't have to undo when we pull in a CEL update. I have a hard time imagining someone taking a dependency on the exact output of the current %e formatter except in a test

pacoxu added a commit to pacoxu/kubernetes that referenced this issue Mar 21, 2025
pacoxu added a commit to pacoxu/kubernetes that referenced this issue Mar 21, 2025
pacoxu added a commit to pacoxu/kubernetes that referenced this issue Mar 21, 2025
liggitt pushed a commit to liggitt/kubernetes that referenced this issue Mar 27, 2025
ahrtr added a commit to ahrtr/kubernetes that referenced this issue Mar 28, 2025
ahrtr added a commit to ahrtr/kubernetes that referenced this issue Mar 31, 2025
k8s-publishing-bot pushed a commit to kubernetes/apiextensions-apiserver that referenced this issue Apr 1, 2025
google/cel-go#1100

Signed-off-by: Benjamin Wang <[email protected]>

Kubernetes-commit: 24536987d89b4537e9254ec815255225504a211b
Heniland pushed a commit to Heniland/kubernetes that referenced this issue May 18, 2025
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

No branches or pull requests

3 participants