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 number stored as null #388

Closed
jdumas opened this issue Dec 9, 2016 · 9 comments
Closed

nan number stored as null #388

jdumas opened this issue Dec 9, 2016 · 9 comments

Comments

@jdumas
Copy link

jdumas commented Dec 9, 2016

Hi there,

Taking the example basic_json__CompatibleNumberFloatType.cpp from the documentation, if I add the following on L16:

double x = j_nan;

Then I get a std::domain_error at the execution. I reckon a nan which is a double should be stored as is in the json. If I want to store a null I can put a nullptr myself. Is there a particular reason this is not the case? I wouldn't mind having a method .set<T>(1.0/0.0) which explicitly stores a nan without converting it to another type.

@nlohmann
Copy link
Owner

nlohmann commented Dec 9, 2016

This seems related to #329.

@nlohmann
Copy link
Owner

nlohmann commented Jan 2, 2017

You are right - I think it should be OK for basic_json to store NAN - only parsing NaN or similar should yield a syntax error. In any case, serializing NAN will result in null.

Any thoughts on this?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Jan 2, 2017
@nlohmann nlohmann modified the milestones: Release 2.0.11, Release 3.0.0 Jan 2, 2017
@jdumas
Copy link
Author

jdumas commented Jan 2, 2017

I don't know. nan and infty are regular IEEE 754 double numbers, as well as signed zeros and subnormals. Since json is an ascii file format, I can expect some loss regarding signed zeros and subnormals, but it should still be possible to serialize/deserialize nan and ±infty numbers. It should be up to the user to decide whether it's an error to have nan and ±infty somewhere in his program.

@jaredgrubb
Copy link
Contributor

The JSON standard does not provide a way to store all IEEE 754 doubles. Adding support would be an extension (a desirable one), which this library has a policy of not doing.

It sucks that basic_json would store something (nan) that it would not serialize correctly (ie, that it would not round-trip), but making the double-constructor throw is even worse. Are there already cases where the serializer would throw such that serializing nan could throw?

@jdumas
Copy link
Author

jdumas commented Jan 3, 2017

Hmm, true if nan and infty are not valid JSON according to the standard, then it sucks =/. Apparently they are allowed in JSON5 however. I know support for JSON5 is not on the table in the foreseeable future, but then it becomes a philosophical choice (very strict behavior and better json compatibility, or more flexible parsing and less painful json experience).

@nlohmann
Copy link
Owner

nlohmann commented Jan 3, 2017

I had a look how Python is handling this. There exists a special flag:

If allow_nan is false (default: True), then it will be a ValueError to serialize out of range float values (nan, inf, -inf) in strict compliance of the JSON specification. If allow_nan is true, their JavaScript equivalents (NaN, Infinity, -Infinity) will be used.

They have two modes:

  • produce invalid JSON by allowing special strings for NaN, etc.
  • throwing when NaN occurs

I do not like the first approach (and also JSON5), because it goes beyond RFC 7159.

On the other hand, the serializer does not throw so far.

@nlohmann
Copy link
Owner

(#329 and #388 are related).

nlohmann added a commit that referenced this issue Mar 12, 2017
- If an overflow occurs during parsing a number from a JSON text, an
exception (std::out_of_range for the moment, to be replaced by a
user-defined exception #244) is thrown so that the overflow is detected
early and roundtripping is guaranteed.
- NaN and INF floating-point values can be stored in a JSON value and
are not replaced by null. That is, the basic_json class behaves like
double in this regard (no exception occurs). However, NaN and INF are
serialized to “null”.
- Adjusted test cases appropriately.
@nlohmann nlohmann added solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed state: please discuss please discuss the issue or vote for your favorite option labels Mar 12, 2017
@nlohmann
Copy link
Owner

This issue is fixed with 8feaf8d as follows:

  • If an overflow occurs during parsing a number from a JSON text, an exception (std::out_of_range for the moment, to be replaced by a user-defined exception Use user-defined exceptions #244) is thrown so that the overflow is detected early and roundtripping is guaranteed.
  • NaN and INF floating-point values can be stored in a JSON value and are not replaced by null. That is, the basic_json class behaves like double in this regard (no exception occurs). However, NaN and INF are serialized to “null”.
  • Adjusted test cases appropriately.

Waiting for Travis to complete. Then this issue can be closed and the different semantics can be described in the wiki.

@nlohmann nlohmann removed the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 12, 2017
@nlohmann nlohmann self-assigned this Mar 19, 2017
@xloem
Copy link

xloem commented Jan 30, 2021

in obverse to this, it would be really nice if get_arithmetic_value convert null to NaN or to some configurable value instead of raising an exception.

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

No branches or pull requests

4 participants