Skip to content

fix: unify NaN error message casing to match go-jsonnet - #1014

Closed
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/nan-overflow-casing
Closed

He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/nan-overflow-casing

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Capitalize all 7 NaN error messages in MathModule.scala from "not a number" to "Not a number" to match go-jsonnet's makeDoubleCheck convention
  • Eliminates internal inconsistency: the Evaluator and Val.Num already used uppercase, but std.sqrt/std.pow/std.asin/std.acos/std.log/std.log2/std.log10 used lowercase
  • Update 6 golden files and StdMathTests.scala assertions

Cross-implementation comparison

Failure mode sjsonnet (before) sjsonnet (after) go-jsonnet 0.22.0 jrsonnet 0.5.0 C++ jsonnet 0.22.0
std.pow(-1, 0.5) NaN "not a number" "Not a number" "Not a number" "non-finite" "not a number"
std.sqrt(-1) NaN "not a number" "Not a number" "Not a number" "out of bounds" "not a number"
std.asin(2) NaN "not a number" "Not a number" "Not a number" "non-finite" "not a number"
std.acos(2) NaN "not a number" "Not a number" "Not a number" "non-finite" "not a number"
std.log(-1) NaN "not a number" "Not a number" "Not a number" "non-finite" "not a number"
std.log2(-1) NaN "not a number" "Not a number" "Not a number" "non-finite" "not a number"
std.log10(-1) NaN "not a number" "Not a number" "Not a number" "non-finite" "not a number"

Test plan

  • ./mill sjsonnet.jvm[3.3.7].test passes (33/33 tests)
  • ./mill sjsonnet.jvm[2.13.18].test passes (8/8 tests)
  • 6 golden files regenerated

Motivation:
MathModule.scala used lowercase "not a number" for NaN errors in
std.sqrt, std.pow, std.asin, std.acos, std.log, std.log2, and
std.log10, while the Evaluator and Val.Num already used uppercase
"Not a number" / "Overflow". This created an inconsistency within
sjsonnet itself and diverged from go-jsonnet's makeDoubleCheck
convention.

Modification:
- MathModule.scala: capitalize "not a number" → "Not a number" in
  all 7 NaN error messages (sqrt, pow, asin, acos, log, log2, log10).
- Update 6 golden files and StdMathTests.scala assertions.

Result:
Every numeric NaN error in sjsonnet now emits "Not a number" with
consistent capitalization, matching go-jsonnet.

Cross-implementation comparison:
| Failure mode              | sjsonnet (before)  | sjsonnet (after)   | go-jsonnet 0.22.0  | jrsonnet 0.5.0     | C++ jsonnet 0.22.0 |
|---------------------------|--------------------|--------------------|--------------------|--------------------|--------------------|
| std.pow(-1, 0.5) NaN      | "not a number"     | "Not a number"     | "Not a number"     | "non-finite"       | "not a number"     |
| std.sqrt(-1) NaN          | "not a number"     | "Not a number"     | "Not a number"     | "out of bounds"    | "not a number"     |
| std.asin(2) NaN           | "not a number"     | "Not a number"     | "Not a number"     | "non-finite"       | "not a number"     |
| std.acos(2) NaN           | "not a number"     | "Not a number"     | "Not a number"     | "non-finite"       | "not a number"     |
| std.log(-1) NaN           | "not a number"     | "Not a number"     | "Not a number"     | "non-finite"       | "not a number"     |
| std.log2(-1) NaN          | "not a number"     | "Not a number"     | "Not a number"     | "non-finite"       | "not a number"     |
| std.log10(-1) NaN         | "not a number"     | "Not a number"     | "Not a number"     | "non-finite"       | "not a number"     |
@He-Pin
He-Pin marked this pull request as draft June 24, 2026 02:43
@He-Pin He-Pin closed this Jun 24, 2026
@He-Pin

He-Pin commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Let's just keep it.

@He-Pin
He-Pin deleted the fix/nan-overflow-casing branch June 24, 2026 03:04
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.

1 participant