msg159367 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2012-08-23 17:22 |
See also #13212.
|
msg168952 - (view) |
Author: Georg Brandl (georg.brandl) * |
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) * |
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) * |
Date: 2012-08-24 17:55 |
Ok, thank you Chris!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:29 | admin | set | github: 58879 |
2012-08-24 17:55:09 | pitrou | set | status: open -> closed resolution: fixed messages:
+ msg169060
stage: commit review -> resolved |
2012-08-24 17:50:33 | python-dev | set | nosy:
+ python-dev messages:
+ msg169058
|
2012-08-24 17:29:42 | pitrou | set | nosy:
+ pitrou messages:
+ msg169053
|
2012-08-24 07:34:55 | cvrebert | set | files:
+ json.rst.patch
messages:
+ msg168976 |
2012-08-23 18:04:32 | georg.brandl | set | nosy:
+ georg.brandl messages:
+ msg168952
|
2012-08-23 17:22:30 | eric.araujo | set | messages:
+ msg168949 |
2012-08-23 10:58:02 | petri.lehtinen | set | nosy:
+ petri.lehtinen messages:
+ msg168935
|
2012-07-20 17:06:21 | cvrebert | set | messages:
+ msg165947 |
2012-06-25 04:45:40 | eric.araujo | set | messages:
+ msg163888 |
2012-06-13 20:01:51 | cvrebert | set | files:
- json.rst.patch |
2012-06-13 20:01:11 | cvrebert | set | files:
+ json.rst.patch
messages:
+ msg162726 |
2012-06-13 19:58:38 | cvrebert | set | files:
- json.rst.patch |
2012-05-22 01:00:14 | cvrebert | set | files:
+ json.rst.patch
messages:
+ msg161310 |
2012-05-21 21:14:11 | eric.araujo | set | title: 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:29 | eric.araujo | set | messages:
+ msg161298 |
2012-05-20 22:50:03 | cvrebert | set | messages:
+ msg161239 |
2012-05-16 07:55:00 | cvrebert | set | files:
- json.rst.patch |
2012-05-16 07:53:57 | cvrebert | set | files:
- json.rst.patch |
2012-05-16 07:53:44 | cvrebert | set | files:
+ json.rst.patch |
2012-05-16 00:24:52 | cvrebert | set | files:
+ json.rst.patch
messages:
+ msg160781 |
2012-05-15 18:55:01 | cvrebert | set | messages:
+ msg160750 |
2012-05-15 09:29:10 | serhiy.storchaka | set | messages:
+ msg160699 |
2012-05-15 09:16:36 | serhiy.storchaka | set | messages:
+ 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:33 | cvrebert | set | messages:
+ 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:02 | cvrebert | set | files:
+ json.rst.patch
messages:
+ msg160695 |
2012-05-15 08:26:30 | ezio.melotti | set | messages:
+ msg160694 |
2012-05-15 08:22:11 | cvrebert | set | messages:
+ msg160693 |
2012-05-15 07:52:22 | cvrebert | set | files:
+ json.rst.patch
messages:
+ msg160692 |
2012-05-15 03:06:39 | cvrebert | set | files:
+ json.rst.patch |
2012-05-15 02:34:45 | cvrebert | set | files:
+ json.rst.patch
nosy:
+ cvrebert messages:
+ msg160677
keywords:
+ patch |
2012-05-06 17:26:46 | ezio.melotti | set | nosy:
- Nurhusien2
|
2012-05-06 17:26:19 | ezio.melotti | set | messages:
- msg160098 |
2012-05-06 17:26:14 | ezio.melotti | set | messages:
- msg160089 |
2012-05-06 17:19:29 | Nurhusien2 | set | messages:
+ msg160098 |
2012-05-06 16:22:49 | Nurhusien2 | set | nosy:
+ Nurhusien2 messages:
+ msg160089
|
2012-05-06 14:17:07 | ezio.melotti | set | nosy:
+ ezio.melotti
versions:
+ Python 2.7, Python 3.2 |
2012-04-27 19:35:50 | eric.araujo | set | keywords:
+ easy nosy:
+ eric.araujo
stage: needs patch |
2012-04-26 14:15:11 | serhiy.storchaka | create | |