Repository: ultrajson/ultrajson (language: python)

Issue:
Infinity handling differs to python native json
It looks like ujson handles infinity different to the native json encoder:

``` python
import ujson, json, numpy

a = np.array([1,2,3,4,numpy.inf])
b = json.dumps({"test" : a.tolist()})
# outputs '{"test": [1,2,3,4,5, Infinity]}'
b= ujson.dumps({"test" : a.tolist()})
# raises OverflowError: Invalid Inf value when encoding double
```

Similarly, an Overflow error is raised when NaN is encountered. It seems the relevant lines are 499-508 in [ultrajsonenc.c](https://github.com/esnme/ultrajson/blob/master/lib/ultrajsonenc.c#L499):

``` c
    if (value == HUGE_VAL || value == -HUGE_VAL)
    {
        SetError (obj, enc, "Invalid Inf value when encoding double");
        return FALSE;
    }
    if (! (value == value)) 
    {
        SetError (obj, enc, "Invalid Nan value when encoding double");
        return FALSE;
    }
```

In the interests of keeping ujson as a drop-in replacement for json, may I suggest this is changed so that it converts the NaN / Infinity to strings (like json) and doesn't raise an error? The decoder would likely need to be changed too...

Cheers
Dan



Discussion / hints:
The specification on json.org is very clear about numeric types only being numbers or exponents.
//JT

From: Danny Price [mailto:notifications@github.com]
Sent: den 19 april 2013 07:59
To: esnme/ultrajson
Subject: [ultrajson] Infinity handling differs to python native json (#80)

It looks like ujson handles infinity different to the native json encoder:

import ujson, json, numpy

a = np.array([1,2,3,4,numpy.inf])

b = json.dumps({"test" : a.tolist()})

# outputs '{"test": [1,2,3,4,5, Infinity]}'

b= ujson.dumps({"test" : a.tolist()})

# raises OverflowError: Invalid Inf value when encoding double

Similarly, an Overflow error is raised when NaN is encountered. It seems the relevant lines are 499-508 in ultrajsonenc.chttps://github.com/esnme/ultrajson/blob/master/lib/ultrajsonenc.c#L499:

```
if (value == HUGE_VAL || value == -HUGE_VAL)

{

    SetError (obj, enc, "Invalid Inf value when encoding double");

    return FALSE;

}

if (! (value == value))

{

    SetError (obj, enc, "Invalid Nan value when encoding double");

    return FALSE;

}
```

In the interests of keeping ujson as a drop-in replacement for json, may I suggest this is changed so that it converts the NaN / Infinity to strings (like json) and doesn't raise an error? The decoder would likely need to be changed too...

Cheers
Dan

—
Reply to this email directly or view it on GitHubhttps://github.com/esnme/ultrajson/issues/80.

Good point - for anyone else interested there's a discussion on [StackOverflow](http://stackoverflow.com/questions/1423081/json-left-out-infinity-and-nan-json-status-in-ecmascript/9218955#9218955) of exactly this. I find it bemusing that JSON cannot fully represent IEE754 floating point numbers!

However, if ujson is intended as a drop-in replacement for python's json, I maintain that handling inf/nans in the exact same way would be preferable to raising an exception. So, may I instead request for an argument [allow_nan](http://docs.python.org/2/library/json.html) for loading/dumping:
_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, instead of using the JavaScript equivalents (NaN, Infinity, -Infinity)._
- Dan

I think this should be solved in a more generic way by a allowing a custom encoder/decoder that handles unknown values.

Adding an option for every special case where someone is abusing the JSON-spec is not the way to go IMO.

I would argue that it's not "abuse" of the JSON spec, but is instead adding extra functionality and compatibility -- already included in simplejson, json, and cjson. Having an identical featureset to the standard json would be fantastic, and I really think handling IEEE floating point is a pretty solid move.

Well, you are asking a JSON encoder to output invalid JSON...

Also:
http://en.wikipedia.org/wiki/Robustness_principle

So following the robustness principle, uJson should accept nan and inf on the decode ("code that receives input should accept non-conformant input as long as the meaning is clear"), but shouldn't encode it ("code that sends commands or data to other machines should conform completely to the specifications")?

While this is obviously not what I would prefer, I do see the advantages of following design principles. I'll leave the ball in your court on this one...

Feel free to contribute a patch towards the robustness principle. Closing this as an issue 

+1 for encoding `inf` and `nan` to JavaScript values `Infinity` and `NaN` .. it seems to me that the JSON specification is simply insufficient by omitting the full range of _IEE754 floating point numbers_. Following specs religiously is **not** the way to go for practical programming.

Have to agree strongly with @hsk81, and the robustness principle is clear on this too. You SHOULD decode things when the meaning is clear, and you SHOULD NOT encode things against the spec. In programming the meaning of SHOULD and MUST are distinct. (And now I shall use it in practice). You SHOULD NOT follow a rule containing the word SHOULD, when it introduces unnecessary complexity into a fundamental part of many people's code. 
We do appear to have a bit of a deviation here. `ujson` writes infinity as `Inf` rather than `Infinity` which mismatches JavaScript, Python's `json` and `ujson`'s own parser so it can't even read back its own JSON!

```python
>>> import math
>>> import ujson
>>> import json

>>> json.dumps([math.nan, math.inf, -math.inf])
'[NaN, Infinity, -Infinity]'
>>> ujson.dumps([math.nan, math.inf, -math.inf])
'[NaN,Inf,-Inf]'

>>> ujson.loads('[NaN,Inf,-Inf]')
ujson.JSONDecodeError: Unexpected character found when decoding 'Infinity'
>>> ujson.loads('[NaN, Infinity, -Infinity]')
[nan, inf, -inf]
```

And on the topic of JSON's inclusion/exclusion of non finite floats, one of the original JSON authors said that he didn't add it to the spec only because he didn't expect anyone to need it and that if anyone found a good use for them then he would consider his own argument to be moot. 


Fix patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index bd0c8159..5a23a231 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -39,10 +39,11 @@ repos:
       - id: trailing-whitespace
         exclude: "^.github/.*_TEMPLATE.md"
 
-  - repo: https://github.com/asottile/setup-cfg-fmt
-    rev: v1.20.1
-    hooks:
-      - id: setup-cfg-fmt
+# Disabled due to asottile/setup-cfg-fmt#73. Re-enable once that issue is fixed.
+#  - repo: https://github.com/asottile/setup-cfg-fmt
+#    rev: v1.20.1
+#    hooks:
+#      - id: setup-cfg-fmt
 
 ci:
   autoupdate_schedule: quarterly
diff --git a/python/objToJSON.c b/python/objToJSON.c
index 96e01ef1..7a063d90 100644
--- a/python/objToJSON.c
+++ b/python/objToJSON.c
@@ -887,7 +887,7 @@ PyObject* objToJSON(PyObject* self, PyObject *args, PyObject *kwargs)
 
   if (encoder.allowNan)
   {
-    csInf = "Inf";
+    csInf = "Infinity";
     csNan = "NaN";
   }
 
diff --git a/python/ujson.c b/python/ujson.c
index c2f37560..62e444ba 100644
--- a/python/ujson.c
+++ b/python/ujson.c
@@ -58,7 +58,7 @@ PyObject* JSONDecodeError;
 #define ENCODER_HELP_TEXT "Use ensure_ascii=false to output UTF-8. " \
     "Set encode_html_chars=True to encode < > & as unicode escape sequences. "\
     "Set escape_forward_slashes=False to prevent escaping / characters." \
-    "Set allow_nan=False to raise an exception when NaN or Inf would be serialized." \
+    "Set allow_nan=False to raise an exception when NaN or Infinity would be serialized." \
     "Set reject_bytes=True to raise TypeError on bytes."
 
 static PyMethodDef ujsonMethods[] = {
diff --git a/setup.cfg b/setup.cfg
index af02ca55..813c224c 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -5,7 +5,7 @@ long_description = file: README.md
 long_description_content_type = text/markdown
 url = https://github.com/ultrajson/ultrajson
 author = Jonas Tarnstrom
-license_file = LICENSE.txt
+license_files = LICENSE.txt
 platforms = any
 classifiers =
     Development Status :: 5 - Production/Stable


Test patch:
diff --git a/tests/test_ujson.py b/tests/test_ujson.py
index 96591c0d..64e4089e 100644
--- a/tests/test_ujson.py
+++ b/tests/test_ujson.py
@@ -842,8 +842,8 @@ def test_encode_no_assert(test_input):
         (1.0, "1.0"),
         (OrderedDict([(1, 1), (0, 0), (8, 8), (2, 2)]), '{"1":1,"0":0,"8":8,"2":2}'),
         ({"a": float("NaN")}, '{"a":NaN}'),
-        ({"a": float("inf")}, '{"a":Inf}'),
-        ({"a": -float("inf")}, '{"a":-Inf}'),
+        ({"a": float("inf")}, '{"a":Infinity}'),
+        ({"a": -float("inf")}, '{"a":-Infinity}'),
     ],
 )
 def test_encode(test_input, expected):