classification
Title: Replace "== None/True/False" with "is"
Type: Stage:
Components: None Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: benjamin.peterson Nosy List: amaury.forgeotdarc, belopolsky, benjamin.peterson, calvin, georg.brandl, rhettinger
Priority: normal Keywords: patch

Created on 2008-03-28 11:06 by calvin, last changed 2008-03-29 15:25 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
0001-Replace-None-True-False-with-is.patch calvin, 2008-03-29 09:17
Messages (11)
msg64628 - (view) Author: Bastian Kleineidam (calvin) Date: 2008-03-28 11:06
Test equality with None/True/False singletons should be done
by "is" rather than "==" to be on the safe side. Otherwise
objects overriding __eq__ could compare equal to one of those
singletons.
msg64631 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-03-28 12:13
You are right of course, but just out of curiosity, do you really have
objects that compare equal to None?
msg64632 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-03-28 12:14
I'm in favor of this patch. Not only is "is" faster here, but it is also
way more idiomatic.
msg64637 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-03-28 13:24
Yes, PEP8 says::

    Comparisons to singletons like None should always be done with
    'is' or 'is not', never the equality operators.

Reading the patch:
- a change modifies "x == False" into "not x", another moves some lines.
I checked that they are OK (x is already the result of a comparison).
- some occurrences of "x != None" are not replaced. Why? (ex. in
test_ast.py)
msg64639 - (view) Author: Bastian Kleineidam (calvin) Date: 2008-03-28 13:46
Amaury, I never saw an object comparing equal to None.
I think the most likely case is a buggy x.__eq__() implementation. Then
the "if x == None" statement gets triggered, and somebody has a hard
time with bug hunting.

Just a note: I used an adapted source checker for this patch, which
initially came from the Sphinx documentation project, found under
tools/check_sources.py. So the credits for this go to Georg Brandl.

That is also why '!=' did not get replaced (the source checker only
searched for '=='. I will post an updated patch.
msg64640 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-28 14:04
Despite the title, the patch replaces "result == False" with "not 
result" rather than "result is False".  While probably ok in this 
particular context, this changes the logic.  For example,

>>> result = ""
>>> result == False
False
>>> not result
True
msg64686 - (view) Author: Bastian Kleineidam (calvin) Date: 2008-03-29 09:17
Here is an updated patch, using "is False" to be consistent, and also
replacing the "!=" occurences.
msg64687 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-29 09:47
This patch is fine.

Before applying, check the code in PyShell to see if the "if response 
is False" line can be simplified to "if not response".
msg64688 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-03-29 09:53
Benjamin, do you want to apply this? Add a Misc/NEWS item as well.
msg64690 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-29 10:08
Don't think changes like this warrant a NEWS entry.  It's a code clean-
up, not a semantic change.
msg64702 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-29 15:25
Patch was committed in r62043. Wummel, thanks for the patch! Georg,
thanks for the practice.
History
Date User Action Args
2008-03-29 15:25:07benjamin.petersonsetstatus: open -> closed
messages: + msg64702
2008-03-29 10:08:15rhettingersetmessages: + msg64690
2008-03-29 09:53:47georg.brandlsetassignee: benjamin.peterson
messages: + msg64688
nosy: + benjamin.peterson
2008-03-29 09:47:31rhettingersetnosy: + rhettinger
resolution: accepted
messages: + msg64687
2008-03-29 09:18:27calvinsetfiles: - 0001-Replace-None-True-False-with-is.patch
2008-03-29 09:17:58calvinsetfiles: + 0001-Replace-None-True-False-with-is.patch
messages: + msg64686
2008-03-28 14:04:32belopolskysetnosy: + belopolsky
messages: + msg64640
2008-03-28 13:46:54calvinsetmessages: + msg64639
2008-03-28 13:24:32amaury.forgeotdarcsetmessages: + msg64637
2008-03-28 12:14:53georg.brandlsetnosy: + georg.brandl
messages: + msg64632
2008-03-28 12:13:23amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg64631
2008-03-28 11:06:49calvincreate