classification
Title: Link to & explain deviations from RFC 4627 in json module docs
Type: enhancement Stage: resolved
Components: Documentation Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: cvrebert, docs@python, eric.araujo, ezio.melotti, georg.brandl, petri.lehtinen, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2012-04-26 14:15 by serhiy.storchaka, last changed 2012-08-24 17:55 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
json.rst.patch cvrebert, 2012-05-15 08:33 per Ezio's review comments review
json.rst.patch cvrebert, 2012-05-16 00:24 reorganize, drop serializer/deserializer division review
json.rst.patch cvrebert, 2012-05-22 01:00 hopefully the final version review
json.rst.patch cvrebert, 2012-06-13 20:01 fix var name inconsistency in 1 example review
json.rst.patch cvrebert, 2012-08-24 07:34 replace "scalar" terminology
Messages (24)
msg159367 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-04-26 14:15
The json module documentation should give a link to RFC 4627 and explain the current implementation is different from it. For example, according to RFC 4627 only an object or an array can be top-level JSON element.
msg160677 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-15 02:34
Draft docs patch. More cross-referencing. Analysis of deviations from RFC based on previous docs & the RFC.

Don't know if there are relevant, more precise implementation limitations that need to be mentioned.

If backported to 2.x, will need to cover encodings slightly more.
msg160692 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-15 07:52
New revision per Éric's Rietveld feedback.

Sidenote: Is there any way to get notified of these reviews? I only saw it because I happened to click the "review" link on a lark.
msg160693 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-15 08:22
The "import json"s were left for uniformity with the other code samples in the module's docs.


Also, here's what the pedantically-strict recipes might look like:

def _reject_inf_nan(string):
    if string in {'-Infinity', 'Infinity', 'NaN'}:
        raise ValueError("JSON does not allow infinite or NaN number values")

def _reject_dupe_keys(pairs):
    obj = {}
    for key, value in pairs:
        if key in pairs:
            raise ValueError("Name %s repeated in an object" % repr(key))
        obj[key] = value
    return obj

def strict_loads(string):
    result = loads(string, parse_constant=_reject_inf_nan, object_pairs_hook=_reject_dupe_keys)
    if not isinstance(result, (dict, list)):
        raise ValueError("The top-level entity of the JSON text was not an object or an array")
    return result


def strict_dumps(obj):
    if not isinstance(obj, (dict, list)):
        raise TypeError("The top-level object of a JSON text must be a dict or a list")
    return dumps(obj, allow_nan=False)
msg160694 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-05-15 08:26
Thanks for the patch, I left more comments on the review page.

IMHO it would be better to list the differences in a bullet list and expand later, rather than having a section for the parser and one for the generator.
AFAIU the differences are:
 * top-level scalar values are accepted/generated;
 * inf and nan are accepted/generated;
 * unicode strings (rather than utf-8) are produced/consumed;
 * duplicate keys are accepted, and the only the last one is used;

You can then add examples and explain "workarounds", either inline or after the list (I don't think it's necessary to add the snippets you posted in the last message though).


> Sidenote: Is there any way to get notified of these reviews?

In theory you should get notifications, in practice it doesn't always work.  We are still working on make it better.
msg160695 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-15 08:33
Further copyediting.
msg160696 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-15 08:40
Reflect broader scope
msg160698 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-05-15 09:16
>     for key, value in pairs:
>         if key in pairs:

"if key in obj:"?
msg160699 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-05-15 09:29
IMHO, it would be sufficient to have a simple bullet list of differences
and notes or warnings in places where Python can generate non-standard
JSON (top-level scalars, inf and nan, non-utf8 encoded strings).
msg160750 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-15 18:55
>>     for key, value in pairs:
>>         if key in pairs:
> 
> "if key in obj:"?

Yes, obviously. :-)  It wrote those very late at night.

@Ezio: These were per Éric's feedback but would be for a separate bug/patch.
msg160781 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-16 00:24
Notification of any future Rietveld comments would be appreciated.
msg161239 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-20 22:50
So, does the refactored patch need any further revising, or is it good to go?
msg161298 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-21 21:13
A message should have been sent to pybugs@rebertia.com.
msg161310 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-22 01:00
Hopefully final revision. Thanks for the quick response Éric!

Changes:
- Cover `ensure_ascii` parameter per latest review comment
- Add enhanced "Character Encodings" section for 2.x backport
-- Cover `encoding` parameter & restrictions
-- Cover non-implementation of encoding detection feature
- Hyperlink the JSON-RPC mention
msg162726 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-06-13 20:01
Any further comments now that the matter of encodings is covered more thoroughly?
msg163888 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-06-25 04:45
This looks good to me, apart from very minor style things that I can change before committing.  Ezio, any last comments?

One thing I haven’t done is comparing the length of the new section to the rest of the file to see if it’s a small or big addition; if it’s too big, that would be distracting.
msg165947 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-07-20 17:06
Pinging on this, since it's been just short of a month since Éric's "last call" comment.
msg168935 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-08-23 10:58
Using the word "scalar" sounds wrong to me. Are strings really considered scalars in Python? At least RFC 4627 doesn't talk about scalars.


+Since the RFC permits RFC-compliant parsers to accept input texts that are not
+RFC-compliant, this module's deserializer is technically RFC-compliant under
+default settings.

This might scare users. Maybe it should note that there are parameters that can be used to turn off the most "harmful" extra features (namely the Inf and NaN support).

Otherwise, the additions look very good and important.
msg168949 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-08-23 17:22
See also #13212.
msg168952 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-08-23 18:04
Neither json.org nor RFC 4627 mention "scalar".  I don't think we should introduce that term, with the necessary ambiguity given the context, needlessly.
msg168976 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-08-24 07:34
Revised patch yet again to instead speak of "non-object, non-array" values and "JSON null, boolean, number, or string" values.

Re: Petri, the patch already mentions the specific parameters one can use to get stricter behavior, albeit not in that particular short paragraph. Feel free to edit the patch.


Any hope of getting this applied soon-ish? I began drafting this over 3 months ago, and yet this issue is tagged "easy"...
msg169053 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-24 17:29
Chris, I'm gonna take a look if nobody beats me to it.
msg169058 - (view) Author: Roundup Robot (python-dev) Date: 2012-08-24 17:50
New changeset 132886ef135d by Antoine Pitrou in branch '3.2':
Issue #14674: Add a discussion of the json module's standard compliance.
http://hg.python.org/cpython/rev/132886ef135d

New changeset 16c0e26fc9cd by Antoine Pitrou in branch 'default':
Issue #14674: Add a discussion of the json module's standard compliance.
http://hg.python.org/cpython/rev/16c0e26fc9cd

New changeset d413b36dbee5 by Antoine Pitrou in branch '2.7':
Issue #14674: Add a discussion of the json module's standard compliance.
http://hg.python.org/cpython/rev/d413b36dbee5
msg169060 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-24 17:55
Ok, thank you Chris!
History
Date User Action Args
2012-08-24 17:55:09pitrousetstatus: open -> closed
resolution: fixed
messages: + msg169060

stage: commit review -> resolved
2012-08-24 17:50:33python-devsetnosy: + python-dev
messages: + msg169058
2012-08-24 17:29:42pitrousetnosy: + pitrou
messages: + msg169053
2012-08-24 07:34:55cvrebertsetfiles: + json.rst.patch

messages: + msg168976
2012-08-23 18:04:32georg.brandlsetnosy: + georg.brandl
messages: + msg168952
2012-08-23 17:22:30eric.araujosetmessages: + msg168949
2012-08-23 10:58:02petri.lehtinensetnosy: + petri.lehtinen
messages: + msg168935
2012-07-20 17:06:21cvrebertsetmessages: + msg165947
2012-06-25 04:45:40eric.araujosetmessages: + msg163888
2012-06-13 20:01:51cvrebertsetfiles: - json.rst.patch
2012-06-13 20:01:11cvrebertsetfiles: + json.rst.patch

messages: + msg162726
2012-06-13 19:58:38cvrebertsetfiles: - json.rst.patch
2012-05-22 01:00:14cvrebertsetfiles: + json.rst.patch

messages: + msg161310
2012-05-21 21:14:11eric.araujosettitle: Add link to RFC 4627 from json documentation -> Link to & explain deviations from RFC 4627 in json module docs
stage: needs patch -> commit review
2012-05-21 21:13:29eric.araujosetmessages: + msg161298
2012-05-20 22:50:03cvrebertsetmessages: + msg161239
2012-05-16 07:55:00cvrebertsetfiles: - json.rst.patch
2012-05-16 07:53:57cvrebertsetfiles: - json.rst.patch
2012-05-16 07:53:44cvrebertsetfiles: + json.rst.patch
2012-05-16 00:24:52cvrebertsetfiles: + json.rst.patch

messages: + msg160781
2012-05-15 18:55:01cvrebertsetmessages: + msg160750
2012-05-15 09:29:10serhiy.storchakasetmessages: + msg160699
2012-05-15 09:16:36serhiy.storchakasetmessages: + msg160698
title: Link to & explain deviations from RFC 4627 in json module docs -> Add link to RFC 4627 from json documentation
2012-05-15 08:40:33cvrebertsetmessages: + msg160696
title: Add link to RFC 4627 from json documentation -> Link to & explain deviations from RFC 4627 in json module docs
2012-05-15 08:33:02cvrebertsetfiles: + json.rst.patch

messages: + msg160695
2012-05-15 08:26:30ezio.melottisetmessages: + msg160694
2012-05-15 08:22:11cvrebertsetmessages: + msg160693
2012-05-15 07:52:22cvrebertsetfiles: + json.rst.patch

messages: + msg160692
2012-05-15 03:06:39cvrebertsetfiles: + json.rst.patch
2012-05-15 02:34:45cvrebertsetfiles: + json.rst.patch

nosy: + cvrebert
messages: + msg160677

keywords: + patch
2012-05-06 17:26:46ezio.melottisetnosy: - Nurhusien2
2012-05-06 17:26:19ezio.melottisetmessages: - msg160098
2012-05-06 17:26:14ezio.melottisetmessages: - msg160089
2012-05-06 17:19:29Nurhusien2setmessages: + msg160098
2012-05-06 16:22:49Nurhusien2setnosy: + Nurhusien2
messages: + msg160089
2012-05-06 14:17:07ezio.melottisetnosy: + ezio.melotti

versions: + Python 2.7, Python 3.2
2012-04-27 19:35:50eric.araujosetkeywords: + easy
nosy: + eric.araujo

stage: needs patch
2012-04-26 14:15:11serhiy.storchakacreate