fix to_json for numbers larger than sys.maxsize
State: closed

- [x] closes #34395 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] passes performance benchmarks
- [x] whatsnew entry

Currently this patch causes a significant reduction for a number of the JSON performance benchmarks. A printout of my results is included below.

[json_benchmarks_results.txt](https://github.com/pandas-dev/pandas/files/4704831/json_benchmarks_results.txt)

I'd love to keep working on this if anybody has ideas for making the solution more efficient!

Labels:
name: Bug
name: IO JSON
description: read_json, to_json, json_normalize

Comments:
If we do it in ujson would be around here:

https://github.com/pandas-dev/pandas/blob/1cad9e52ee1028db3bb4b9e86998104eb4d6b0ee/pandas/_libs/src/ujson/python/objToJSON.c#L1636

So instead of raising I think would convert to a buffer or string object and write that out. Would probably need to do something else on top of that to not add quotes in the output
@WillAyd Thanks for this! I'm some of the way along the solution. 

Inside the if statement that starts on line
 https://github.com/pandas-dev/pandas/blob/1cad9e52ee1028db3bb4b9e86998104eb4d6b0ee/pandas/_libs/src/ujson/python/objToJSON.c#L1636
I convert `obj` into a string. This then needs be passed along for encoding. 

Scrolling down `objToJSON.c` I understand that in this line
https://github.com/pandas-dev/pandas/blob/1cad9e52ee1028db3bb4b9e86998104eb4d6b0ee/pandas/_libs/src/ujson/python/objToJSON.c#L2285
the `objToJSON` function has created an encoded string using the `encoder` and `buffer` objects. I feel like I'm not quite clear on how those work here... If I'm understanding correctly, the `encoder` is a `struct` with information about formatting and the `buffer` is where the output is actually stored. In that case I would want to figure out how to set the parameters of the encoder and how to dump my string into the buffer?

Thanks so much for the help with this!
You typically don't need to touch the buffer. At a high level the way it works is you set the type of object being serialized and there is a context associated with that type which the serializer will use to determine whether to continue recursion further (i.e. to keep nesting say a dictionary) or to simply write the object.

You can take any example, but if you look at the float code:

https://github.com/pandas-dev/pandas/blob/d5e8edcdfa0eaf6847faf95e15a607e03068eb4c/pandas/_libs/src/ujson/python/objToJSON.c#L1642

We are introspecting the object, setting the appropriate `doubleValue` member (for the non null case) and telling the serializer that the type of the object is `JT_DOUBLE`. If you then look here you'll see ujson writing that value out to the buffer:

https://github.com/pandas-dev/pandas/blob/d5e8edcdfa0eaf6847faf95e15a607e03068eb4c/pandas/_libs/src/ujson/lib/ultrajsonenc.c#L1072

So for this, would need to figure out a way to write out a numeric value that exceeds C's storage limits for integers. 

One possible way is to store the Python str representation of the value into the `cStr` member of the `TypeContext` struct much like we do for JT_UTF8 types, but instead of using UTF8 as the type (which will add quoting) create a new enum entry here that allows that wouldn't quote the output and add the appropriate code to the serializer for that type

https://github.com/pandas-dev/pandas/blob/d5e8edcdfa0eaf6847faf95e15a607e03068eb4c/pandas/_libs/src/ujson/lib/ultrajson.h#L146


Can you add a test for this? Will help guide the implementation and feedback
> Can you add a test for this? Will help guide the implementation and feedback

I added one in  `pandas/tests/io/json/test_ujson.py`. I'm testing encoding and decoding against the in-built `json` library:

```python
big_num = sys.maxsize + 1
encoding = ujson.dumps(big_num)

assert encoding == json.dumps(big_num)
assert ujson.loads(encoding) == big_num
```
Try putting parentheses around the cast

Sent from my iPhone

> On Jun 10, 2020, at 2:51 PM, Andrew Wieteska <notifications@github.com> wrote:
> 
> ﻿
> @arw2019 commented on this pull request.
> 
> In pandas/_libs/src/ujson/python/objToJSON.c:
> 
> > @@ -2126,6 +2126,21 @@ double Object_getDoubleValue(JSOBJ Py_UNUSED(obj), JSONTypeContext *tc) {
>      return GET_TC(tc)->doubleValue;
>  }
>  
> +const char *Object_getBigNumStringValue(JSOBJ obj, JSONTypeContext *tc, 
> +                                    size_t *_outLen) {
> +    PyObject* repr = PyObject_Str(obj);
> +    const char *str = PyUnicode_AsUTF8AndSize(repr, <Py_ssize_t> *_outLen);
> The compiler still complains here:
> 
> pandas/_libs/src/ujson/python/objToJSON.c:2132:53: error: expected expression before ‘<’ token
>      const char *str = PyUnicode_AsUTF8AndSize(repr, <Py_ssize_t> *_outLen);
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

> Try putting parentheses around the cast

Still complains! I tried both
```
    const char *str = PyUnicode_AsUTF8AndSize(repr, (<Py_ssize_t> *_outLen));
```
and 
```
    const char *str = PyUnicode_AsUTF8AndSize(repr, (<Py_ssize_t>) *_outLen);
```
Just use parentheses not angled brackets so (Py_ssize_t) *_outLen
> Just use parentheses not angled brackets so (Py_ssize_t) *_outLen

Still getting this:
```
pandas/_libs/src/ujson/python/objToJSON.c:2133:53: error: passing argument 2 of ‘PyUnicode_AsUTF8AndSize’ makes pointer from integer without a cast [-Werror=int-conversion]
     const char *str = PyUnicode_AsUTF8AndSize(repr, (Py_ssize_t) *_outLen);
                                                     ^~~~~~~~~~~~~~~~~~~~~
```
Oh sorry got mixed up on comments. You need to pass the pointer and can cast that if needed, so `PyUnicode_AsUTF8AndSize(repr, (Py_ssize_t *) _outLen)`
> You need to pass the pointer and can cast that if needed, so `PyUnicode_AsUTF8AndSize(repr, (Py_ssize_t *) _outLen)`

@WillAyd This works - thanks so much for the help on this!

As you said what's left, modulo memory leaks, is to fix up `pyEncoder`.  I added `Object_getBigNumAsString` to the definition here:
https://github.com/pandas-dev/pandas/blob/5fdd6f50ac5ac2de939ef5b849e4c41e27e623a3/pandas/_libs/src/ujson/python/objToJSON.c#L2177
which however throws an error:
```
pandas/_libs/src/ujson/python/objToJSON.c:2197:9: error: initialization of ‘JSINT32 (*)(void *, JSONTypeContext *)’ {aka ‘int (*)(void *, struct __JSONTypeContext *)’} from incompatible pointer type ‘const char * (*)(void *, JSONTypeContext *, size_t *)’ {aka ‘const char * (*)(void *, struct __JSONTypeContext *, long unsigned int *)’} [-Werror=incompatible-pointer-types]
         Object_getBigNumStringValue,
```
I feel like the issue is that the compiler treats `Object_getBigNumAsStringValue` as undeclared back in line 1638-9 where I do:
```
tc->type = JT_BIGNUM;
pc->PyTypeToUTF8 = Object_getBigNumStringValue;
```
because that's the other compiler error:
```
pandas/_libs/src/ujson/python/objToJSON.c:1639:32: error: ‘Object_getBigNumStringValue’ undeclared (first use in this function)
             pc->PyTypeToUTF8 = Object_getBigNumStringValue;
```
but this doesn't make sense to me because I declare `Object_getBigNumStringValue` in the `ultrajson.h` so in effect there should be a declaration at the top of the file...
IIUC your problem is probably that you aren't updating the struct declaration here:

https://github.com/pandas-dev/pandas/blob/5fdd6f50ac5ac2de939ef5b849e4c41e27e623a3/pandas/_libs/src/ujson/python/objToJSON.c#L119

You can declare that pointer at the end of the struct that way everything between that and the definition you are updating above will know appropriately how to handle `PyObjectEncoder`
Ok! So this is sort of working now, meaning that `ujson.dumps(sys.maxsize+1) == str(sys.maxsize+1)`.  

However the test we added still doesn't pass:
```
================================================================================== short test summary info ==================================================================================
FAILED pandas/tests/io/json/test_ujson.py::TestUltraJSONTests::test_dumps_ints_larger_than_maxsize - OverflowError: Unterminated UTF-8 sequence when encoding string
```
@WillAyd Is it worth opening another issue/PR to address the errors raised by `ujson.loads(sys.maxsize+1)`? I'm down to work on that if yes!
There should already be one if you search the issue tracker. But for sure would take a PR for that as well
I ran the json benchmarks for this branch. Here is the summary:
```
       before           after         ratio
     [208a6ecf]       [44b79f10]
     <master>         <json-Overflow-long-int>
+            175M             202M     1.16  io.json.ReadJSONLines.peakmem_read_json_lines_concat('int')
+            175M             202M     1.16  io.json.ReadJSONLines.peakmem_read_json_lines_concat('datetime')
-         559±8ms         505±10ms     0.90  io.json.ToJSONISO.time_iso_format('index')
-        491±10ms         436±10ms     0.89  io.json.ToJSONISO.time_iso_format('records')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```
So the CI build is failing, with this message:
```
1.07s$ ci/run_tests.sh
xvfb-run pytest -m "(not slow and not network and not clipboard)" -n auto --dist=loadfile -s --strict --durations=30 --junitxml=test-data.xml pandas
ImportError while loading conftest '/home/travis/build/pandas-dev/pandas/pandas/conftest.py'.
pandas/__init__.py:34: in <module>
    raise ImportError(
E   ImportError: C extension: 'dtypes' from partially initialized module 'pandas._libs.tslibs' (most likely due to a circular import) (/home/travis/build/pandas-dev/pandas/pandas/_libs/tslibs/__init__.py) not built. If you want to import pandas from the source directory, you may need to run 'python setup.py build_ext --inplace --force' to build the C extensions first.
The command "ci/run_tests.sh" exited with 4.
```
I feel like the problem could be these lines in `pandas/tests/io/json/test_pandas.py`:
```
        originalSeries = Series(bigNum, dtype=object, index=["articleId"])
```
```
        originalDataFrame = DataFrame(
            bigNum, dtype=object, index=["articleId"], columns=[0]
        )
```
since Travis is complaining about the `dtypes` extension...? 
Can you merge master and repush? I don’t think that error is related to changes made here 

Sent from my iPhone

> On Jun 19, 2020, at 7:11 PM, Andrew Wieteska <notifications@github.com> wrote:
> 
> ﻿
> So the CI build is failing, with this message:
> 
> 1.07s$ ci/run_tests.sh
> xvfb-run pytest -m "(not slow and not network and not clipboard)" -n auto --dist=loadfile -s --strict --durations=30 --junitxml=test-data.xml pandas
> ImportError while loading conftest '/home/travis/build/pandas-dev/pandas/pandas/conftest.py'.
> pandas/__init__.py:34: in <module>
>     raise ImportError(
> E   ImportError: C extension: 'dtypes' from partially initialized module 'pandas._libs.tslibs' (most likely due to a circular import) (/home/travis/build/pandas-dev/pandas/pandas/_libs/tslibs/__init__.py) not built. If you want to import pandas from the source directory, you may need to run 'python setup.py build_ext --inplace --force' to build the C extensions first.
> The command "ci/run_tests.sh" exited with 4.
> I feel like the problem could be these lines in pandas/tests/io/json/test_pandas.py:
> 
>         originalSeries = Series(bigNum, dtype=object, index=["articleId"])
>         originalDataFrame = DataFrame(
>             bigNum, dtype=object, index=["articleId"], columns=[0]
>         )
> since Travis is complaining about the dtypes extension...?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

@jreback Benchmark results are attached.
[json_performance_benchmarks.txt](https://github.com/pandas-dev/pandas/files/4812015/json_performance_benchmarks.txt)

The ones which changed significantly are:
```
       before           after         ratio
     [7d0ee96f]       [9e1b95f7]
     <master>         <json-Overflow-long-int>
+         268±4ms         315±40ms     1.17  io.json.ToJSON.time_to_json('index', 'df_int_floats')
+         242±3ms          278±3ms     1.15  io.json.ToJSON.time_to_json('index', 'df_td_int_ts')
+        364±20ms         407±20ms     1.12  io.json.ToJSON.time_to_json_wide('split', 'df_int_floats')
-        706±30ms         606±10ms     0.86  io.json.ReadJSONLines.time_read_json_lines('int')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
```
I'm working on adding long int benchmarks.
looks ok to me. @WillAyd when you are happy.
Thanks @arw2019 very nice PR
Had a lot of fun doing this. @WillAyd @jreback  thanks so much for all the help!

#34984 is a follow-up PR to deal with the correspondent problem in `json.decode`