fix to_json for numbers larger than sys.maxsize

[arw2019]

• closes BUG: OverflowError on to_json with numbers larger than sys.maxsize #34395
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• passes performance benchmarks
• 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

I'd love to keep working on this if anybody has ideas for making the solution more efficient!

[jreback]

this needs to be handled in the ujson impl

[WillAyd]

If we do it in ujson would be around here:

pandas/pandas/_libs/src/ujson/python/objToJSON.c

Line 1636 in 1cad9e5

if (exc && PyErr_ExceptionMatches(PyExc_OverflowError)) {

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

[arw2019]

@WillAyd Thanks for this! I'm some of the way along the solution.

Inside the if statement that starts on line

pandas/pandas/_libs/src/ujson/python/objToJSON.c

Line 1636 in 1cad9e5

if (exc && PyErr_ExceptionMatches(PyExc_OverflowError)) {

I convert obj into a string. This then needs be passed along for encoding.

Scrolling down objToJSON.c I understand that in this line

pandas/pandas/_libs/src/ujson/python/objToJSON.c

Line 2285 in 1cad9e5

ret = JSON_EncodeObject(oinput, encoder, buffer, sizeof(buffer));

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!

[WillAyd]

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:

pandas/pandas/_libs/src/ujson/python/objToJSON.c

Line 1642 in d5e8edc

} else if (PyFloat_Check(obj)) {

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:

pandas/pandas/_libs/src/ujson/lib/ultrajsonenc.c

Line 1072 in d5e8edc

case JT_DOUBLE: {

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

pandas/pandas/_libs/src/ujson/lib/ultrajson.h

Line 146 in d5e8edc

enum JSTYPES {

[WillAyd]

Can you add a test for this? Will help guide the implementation and feedback

[arw2019]

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:

big_num = sys.maxsize + 1
encoding = ujson.dumps(big_num)

assert encoding == json.dumps(big_num)
assert ujson.loads(encoding) == big_num

[arw2019]

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...?

[WillAyd]

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 ***@***.***> 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]

can you run the json perf tests to see if anything is affected. you may need to add some long ints there as well.

[arw2019]

@jreback Benchmark results are attached.
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.

[arw2019]

@jreback Not sure where we want the longints to go - whether into ints or separately. I would think we also want to make sure we have both positive and negative long ints in there?

[jreback]

this prob fine

[jreback]

this prob fine

[jreback]

looks ok to me. @WillAyd when you are happy.

[WillAyd]

Thanks @arw2019 very nice PR

[arw2019]

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