Skip to content
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

NaN is serialized to NaN should be "null" #395

Closed
Schultzer opened this issue Mar 8, 2019 · 6 comments · Fixed by #407
Closed

NaN is serialized to NaN should be "null" #395

Schultzer opened this issue Mar 8, 2019 · 6 comments · Fixed by #407

Comments

@Schultzer
Copy link

Following the JSON spec NaN and +/- infinity should be serialized to "null"

https://stackoverflow.com/questions/1423081/json-left-out-infinity-and-nan-json-status-in-ecmascript

Finite numbers are stringified as if by calling ToString(number). NaN and Infinity regardless of sign are represented as the String null.
@jacobwilliams
Copy link
Owner

jacobwilliams commented Jun 2, 2019

I'm not sure what you are proposing. NaN and +/- Infinity cannot be represented in JSON. The quote above is from the Javascript spec which isn't the same thing.

My preference would be to parse NaN, +Infinity, -Infinity as the corresponding floating point values, perhaps using some trickery with ieee_positive_inf, ieee_negative_inf, and ieee_quiet_nan.

@zbeekman
Copy link
Contributor

zbeekman commented Jun 3, 2019

An option for signaling NaN would be nice, FWIW.

@Schultzer
Copy link
Author

Schultzer commented Jun 3, 2019

Hi Jacob,

I'm sorry for the confusion.

If I do something like this

type(json_core) :: json
type(json_value), pointer :: root
zero = 0
nan = 0/zero
call json%add(root, 'nan', nan)
call json%print(root, './nan.json')
call json%destroy(root)

I would get {"nan": NaN} instead of {"nan": "null"}

I guess what I'm proposing is to serialize NaN and Infinity into "null" in the output.

Let me know if this cleared things up a bit.

@jacobwilliams
Copy link
Owner

jacobwilliams commented Jun 29, 2019

I guess we could have different options. Keep in mind that it also has to work for parsing. Would it be better to represent it as the JSON null value rather than a string? The options could be:

  • {"nan": "null", "+Infinity": "null", "-Infinity": "null"}
  • {"nan": null, "+Infinity": null, "-Infinity": null}
  • {"nan": NaN, "+Infinity": +Infinity, "-Infinity": -Infinity} -- (this is what JSON5 does. See JSON5 #398) this would require updating the parser to recognize these are floating points and set the value using the appropriate ieee routine.

The problem with the first two is that they aren't reversible. You can add a +Infinity but you can't get it back. The last one is reversable. This could be important if using an array. For example: it seems weird to produce something like this: "x": [1.0, "null", 2.0, 3.0, "null"] from a vector of reals. Maybe 4th slightly better option would be:

  • {"nan": "NaN", "+Infinity": "+Infinity", "-Infinity": "-Infinity"}
    At least then you could tell what they were (also potentially get the same values back..I'd need to check if the string to real routine works for these cases).

@Schultzer
Copy link
Author

Schultzer commented Jun 29, 2019

I feel this is inherently difficult, given the JSON spec, in a perfect world JSON would adhere to ieee floating points, it bit me in the foot, when I found out that JSON did not support ieee.

I'm using your lib in my blas project, but given all these issues, I might just do the serializing from fortran to rust myself, then using JSON as format in between.

I like your idea to have an optional JSON5 parser, but maybe even just documenting that we don't follow spec when it comes to NaN and Infinities is sufficient.

jacobwilliams added a commit that referenced this issue Jul 7, 2019
… -Infinity real values. If these value are encountered, they are serialized as strings (in quotes). See #395
@jacobwilliams jacobwilliams mentioned this issue Jul 8, 2019
@jacobwilliams
Copy link
Owner

Note: see the new initialize() options: null_to_real_mode, non_normal_mode, and use_quiet_nan. They will be in the next release.

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

Successfully merging a pull request may close this issue.

3 participants