classification
Title: json checks True/False by identity, not boolean value
Type: behavior Stage: resolved
Components: Documentation Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: abki, bob.ippolito, bwanamarko, chris.jerdonek, eric.araujo, ezio.melotti, flox, georg.brandl, ggenellina, mark.dickinson, python-dev, r.david.murray, rhettinger, serhiy.storchaka, techtonik, zearin
Priority: normal Keywords: patch

Created on 2009-01-14 05:31 by ggenellina, last changed 2016-06-30 11:04 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
json.diff ggenellina, 2009-01-14 05:48 patch for json package
json_doc_truefalse.patch serhiy.storchaka, 2012-11-25 16:28 Patch for 3.x review
json_doc_truefalse-2.7.patch serhiy.storchaka, 2012-11-25 16:29 Patch for 2.7 review
json_doc_truefalse_2.patch serhiy.storchaka, 2013-12-09 17:52 Patch for 3.4 review
json_doc_truefalse_3.patch serhiy.storchaka, 2016-06-22 16:26 Patch for 3.6 review
json_doc_truefalse-2.7_2.patch serhiy.storchaka, 2016-06-22 16:27 Patch for 2.7 review
Messages (27)
msg79832 - (view) Author: Gabriel Genellina (ggenellina) Date: 2009-01-14 05:31
json compares arguments against True/False by identity, not by boolean 
value; by example:

    if (skipkeys is False and ensure_ascii is True and
        check_circular is True and allow_nan is True ...

Using `ensure_ascii=1` won't work as intended. I don't see the reason 
to check those values by identity - they *are* boolean flags, and 
should be checked by value, as the usual practice.

The attached patch fixes the code and documentation (and a bug encoding 
True/False as keys, including unit tests)
msg108719 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-06-26 12:33
As a patch has been provided can't this be moved forward?  The Python versions also need updating.
msg108749 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-06-26 20:19
Most likely we are waiting for Bob's review.  Have you tested the patch?
msg109175 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-03 11:02
Successfully ran test_json for Python 2.6.5 on Windows Vista.
msg146498 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-27 15:51
I agree with the request.  I don’t know if this behavior change can go into stable versions; Raymond will decide, as Bob seems to have retired from the maintenance of stdlib json.
msg176354 - (view) Author: anatoly techtonik (techtonik) Date: 2012-11-25 13:59
+1 for all branches.

Is that handling of booleans PEPified somewhere?
msg176370 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-25 16:28
Actually most proposed changes were already committed, only the documentation left. Here is an updated and fixed patch for 3.x.
msg176455 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-11-26 21:15
I'm not sure the documentation should be changed.  While boolean values are accepted, user should prefer True/False, so the fact that you can pass other things shouldn't be documented/advertized IMHO.
The patch has a couple of other changes that could be included though.
msg176457 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-26 21:30
Docstrings already changed to "true/false" instead "True/False". Almost, some places were forgotten. The patch mainly changes the ReST documentation to conform docstrings.
msg176471 - (view) Author: anatoly techtonik (techtonik) Date: 2012-11-27 10:50
On Tue, Nov 27, 2012 at 12:15 AM, Ezio Melotti <report@bugs.python.org>wrote:

> I'm not sure the documentation should be changed.  While boolean values
> are accepted, user should prefer True/False, so the fact that you can pass
> other things shouldn't be documented/advertized IMHO.
>

I'd raise that question on python-dev.

> The patch has a couple of other changes that could be included though.
>

Yes, things that fall out of scope of this report should go into other
reports. Otherwise we'll chat here for ages.
msg176473 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-11-27 13:08
> user should prefer True/False

I disagree:  I don't see any reason why the normal duck-typing rules shouldn't hold here.  So +1 for the documentation changes from me.
msg176480 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-11-27 16:43
> I don't see any reason why the normal duck-typing rules shouldn't hold here.

The rules should apply, the only "problem" is in the documentation.

The documentation here could answer two questions:
1) what should I pass to these args to make them work?
2) can I pass things that are not True/False?

For the 1st question the answer is "True or False" (sure you can pass other things, but if you have to pick two values use these).  Regarding the 2nd question I'm not even sure why would anyone ask it.  The only case I can think about is someone used to 0/1 instead of True/False but if I wanted to know if they worked, I would, in order:
1) use True/False because I'm sure it works;
2) convert what I have to True/False because I'm sure it works;
3) try what I have and see if it works (and be paranoid because it might break somewhere);
4) read to code to make sure what I have works;
5) read the documentation and see if it mentions values that are not explicitly True/False.

Moreover even if I read 'true' instead of 'True' in the docs I wouldn't think "it's lowercase so it must accept any true value and not just True/False".  IOW the lowercase "true" only serves to provide a mild reassurance to someone that wants to use something different from True/False, that read the docs expecting to find this reassurance, and that is fine with how mild it is.

OTOH is not a big deal, I was just explaining the reasoning behind my position.  Two more things though: I looked at the docstring of loads/dumps and I see "True", "false" and "is specified";  I don't like much the parallel between the lowercase "is true" and the uppercase "default: False".
msg176489 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-11-27 20:19
On the whole, the patch looks okay to me.  I think we should strive for correctness in the documentation where possible.

While keeping "True" would keep the forward implications correct and document the "preferred" value, there is often an implied assumption (which is sometimes stated) that the negation of the first term does not trigger or guarantee the behavior described in the second term.  It is that second implication that would be incorrect in some cases if "True" is kept (e.g. *check_circular* and *allow_nan*).

In cases where both "if True" and "if False" are explicitly stated, keeping "True" and "False" would mean not documenting certain cases.
msg176634 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-11-29 07:50
Ezio told me my previous comment was hard to understand.  For the record, what I meant was that if you have a statement like, "If *allow_nan* is ``True``, then ....  Otherwise, ...," then this can be read to mean that something like 1 for *allow_nan* would be covered by the otherwise clause (because otherwise means "or else").

Changing "If *allow_nan* is ``True``" to "equals ``True``" would be one way to address this while still stating the preferred value.  But as I told Ezio on IRC, I would be okay with whatever you come up with.
msg176635 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-11-29 07:54
> Changing "If *allow_nan* is ``True``" to "equals ``True``"

I don't think that's any more accurate.  Many Python objects are 'true' but not equal to 'True' (e.g., nonempty collections).
msg176636 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-11-29 07:56
You're right.  My mistake. :)
msg180930 - (view) Author: Zearin (zearin) Date: 2013-01-29 18:59
Note: Javascript has something analogous to Python’s ``==`` and ``is``.  

In JS: 

> 0 == false
true
> 0 === false
false
> 1 == true
true
> 1 === true
false

Perhaps this discrepancy could be fixed in the JSON processing?
msg180936 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-01-29 19:26
I would be very careful trying to reason by analogy there.  ==, is, and javascripts === are rather different in detail, from what I understand.  Nor do I see what javascript has to do with this issue :)

As far as the remaining documentation issue here, IMO to follow the convention used most often in the docs (and docstrings), the text in Chris' example should read:

   If *allow_nan* is true

That is, drop the code markup and capitalization to indicate that any true value is accepted.  A naive user will use True, a non-naive user will understand the implication.  I know Ezio doesn't think this is worthwhile, but I disagree :)
msg181828 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-02-10 18:18
According to the experts index, Bob is no longer actively maintaining the module.
msg205718 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-09 17:43
See also issue19795 which contains much larger patch for other modules.
msg205719 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-09 17:52
Updated to tip.
msg247005 - (view) Author: Mark Mikofski (bwanamarko) Date: 2015-07-20 21:56
This is effecting IronPython as well, because .NET objects return copies not references. If a .NET assembly method is called from IronPython, its return is a copy, not a reference. Therefore the reference of a boolean return is not the same as the internal Python reference for that boolean, and the JSON encoder doesn't recognize the value as a boolean, but instead  treats it as an integer, and returns `str(o)`, which for `True` is "True" and for `False` is "False". Then the decoder can't interpret the JSON object because true and false are capitalize, which is not in its spec. :(

See my comment https://github.com/IronLanguages/main/issues/1033.

The patch would solve this problem. A copy of a boolean will be equal to  its value, ie False == False even if their references are not the same.
msg247015 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-07-21 06:03
This issue is about boolean parameters, not boolean serialized values. Please open new issue for your problem Mark.
msg269077 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-22 16:26
Updated patches are synchronized with current sources and address Ezio's comments (sorry for the delay Ezio, I missed your comments).
msg269453 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-06-28 22:05
These latest two patches look fine.  Go ahead with them.
msg269574 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-30 11:03
New changeset 8f1d3ebdbc56 by Serhiy Storchaka in branch '2.7':
Issue #4945: Improved the documenting of boolean arguments in the json module.
https://hg.python.org/cpython/rev/8f1d3ebdbc56

New changeset 324b061ce220 by Serhiy Storchaka in branch '3.5':
Issue #4945: Improved the documenting of boolean arguments in the json module.
https://hg.python.org/cpython/rev/324b061ce220

New changeset f2d1dba10a0e by Serhiy Storchaka in branch 'default':
Issue #4945: Improved the documenting of boolean arguments in the json module.
https://hg.python.org/cpython/rev/f2d1dba10a0e
msg269575 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-30 11:04
Thanks Raymond.
History
Date User Action Args
2016-06-30 11:04:25serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg269575

stage: patch review -> resolved
2016-06-30 11:03:44python-devsetnosy: + python-dev
messages: + msg269574
2016-06-28 22:05:01rhettingersetmessages: + msg269453
2016-06-22 16:27:17serhiy.storchakasetfiles: + json_doc_truefalse-2.7_2.patch
2016-06-22 16:26:57serhiy.storchakasetfiles: + json_doc_truefalse_3.patch

messages: + msg269077
versions: + Python 3.6, - Python 3.4
2015-07-21 06:03:50serhiy.storchakasetmessages: + msg247015
2015-07-20 21:56:59bwanamarkosetnosy: + bwanamarko
messages: + msg247005
2014-09-05 18:10:00serhiy.storchakasetversions: + Python 3.5, - Python 3.3
2014-02-03 15:43:31BreamoreBoysetnosy: - BreamoreBoy
2013-12-09 17:52:31serhiy.storchakasetfiles: + json_doc_truefalse_2.patch
nosy: + georg.brandl
messages: + msg205719

2013-12-09 17:43:01serhiy.storchakasetmessages: + msg205718
versions: - Python 3.2
2013-02-10 18:18:07mark.dickinsonsetassignee: bob.ippolito ->
messages: + msg181828
2013-01-29 19:26:23r.david.murraysetmessages: + msg180936
2013-01-29 18:59:25zearinsetnosy: + zearin
messages: + msg180930
2012-11-29 07:56:20chris.jerdoneksetmessages: + msg176636
2012-11-29 07:54:07mark.dickinsonsetmessages: + msg176635
2012-11-29 07:50:07chris.jerdoneksetmessages: + msg176634
2012-11-27 20:19:10chris.jerdoneksetmessages: + msg176489
2012-11-27 19:16:11serhiy.storchakasetnosy: + chris.jerdonek
2012-11-27 16:43:19ezio.melottisetmessages: + msg176480
2012-11-27 13:08:52mark.dickinsonsetnosy: + mark.dickinson
messages: + msg176473
2012-11-27 10:50:42techtoniksetmessages: + msg176471
2012-11-26 21:30:32serhiy.storchakasetmessages: + msg176457
2012-11-26 21:15:05ezio.melottisetmessages: + msg176455
2012-11-25 16:29:13serhiy.storchakasetfiles: + json_doc_truefalse-2.7.patch
2012-11-25 16:28:33serhiy.storchakasetfiles: + json_doc_truefalse.patch
versions: + Python 3.4
nosy: + serhiy.storchaka

messages: + msg176370

components: + Documentation, - Library (Lib)
2012-11-25 13:59:59techtoniksetnosy: + techtonik
messages: + msg176354
2011-10-27 15:51:37eric.araujosetnosy: + rhettinger, ezio.melotti, eric.araujo
messages: + msg146498
2011-10-26 18:04:31floxsetnosy: + flox

versions: + Python 3.3, - Python 2.6, Python 3.1
2011-10-26 16:52:02abkisetnosy: + abki
2010-07-03 11:03:54BreamoreBoysetmessages: + msg109175
2010-06-26 20:19:26r.david.murraysetversions: + Python 3.2, - Python 3.0
nosy: + r.david.murray

messages: + msg108749

stage: patch review
2010-06-26 12:33:31BreamoreBoysetnosy: + BreamoreBoy
messages: + msg108719
2009-01-14 17:35:02benjamin.petersonsetassignee: bob.ippolito
nosy: + bob.ippolito
2009-01-14 05:48:06ggenellinasetfiles: + json.diff
keywords: + patch
2009-01-14 05:31:37ggenellinacreate