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
Fix encoding of infinity (#80). #562
Conversation
Codecov Report
@@ Coverage Diff @@
## main #562 +/- ##
=======================================
Coverage 91.58% 91.58%
=======================================
Files 6 6
Lines 1901 1901
=======================================
Hits 1741 1741
Misses 160 160
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ENCODER_HELP_TEXT
in python/ujson.c
should probably get an s/Inf/Infinity/
as well. LGTM otherwise.
GitHub tip: Add something like "Fixes #80" in the PR message and it'll autoclose the issue when merged: |
Infinity was being encoded as 'Inf' which, whilst the JSON spec doesn't include any non-finite floats, differs from the conventions in other JSON libraries, JavaScript of using 'Infinity'. It also differs from what `ujson.loads()` expects so that `ujson.loads(ujson.dumps(math.inf))` raises an exception. Closes ultrajson#80.
Ughh, how do you put up with these precommit hooks. This one essentially reverts us back to the deprecated behaviour I was trying to fix. |
Yeah, if 854597d is a problem, we can remove/comment out |
There we go. Even precommit is happy 🙄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
Infinity was being encoded as 'Inf' which, whilst the JSON spec doesn't include any non-finite floats, differs from the conventions in other JSON libraries, JavaScript of using 'Infinity'. It also differs from what
ujson.loads()
expects so thatujson.loads(ujson.dumps(math.inf))
raises an exception.