Names changed to protect the guilty

A

Aahz

The following line of lightly munged code was found in a publicly
available Python library...

if schema.elements.has_key(key) is False:

Sorry, just had to vent.
 
M

MonkeeSage

The following line of lightly munged code was found in a publicly
available Python library...

Yes, this violates the Holy, Inspired, Infallible Style Guide (pbuh),
which was written by the very finger of God when the world was still in
chaotic darkness. But I guess I'm an atheist when it comes to PEP 8. If
it is clearer to you to make the condition explicit ("blah not False"),
rather than implicit ("not blah"), then use the former. I say write the
code the way *you* (and your team if applicable) are best able to read,
write and maintain it. Then when other people tell you that it isn't
good style, or isn't "pythonic," just stab them in the face with
soldering iron ala Chris Walken. :)

Regards,
Jordan
 
N

Neil Cerutti

Yes, this violates the Holy, Inspired, Infallible Style Guide
(pbuh), which was written by the very finger of God when the
world was still in chaotic darkness. But I guess I'm an atheist
when it comes to PEP 8. If it is clearer to you to make the
condition explicit ("blah not False"), rather than implicit
("not blah"), then use the former. I say write the code the way
*you* (and your team if applicable) are best able to read,
write and maintain it. Then when other people tell you that it
isn't good style, or isn't "pythonic," just stab them in the
face with soldering iron ala Chris Walken. :)

I agree on both points. It's a style issue, and that hidden tests
(taking advantage of how certain objects convert to boolian
values) is harder to read.
 
J

John Machin

MonkeeSage said:
"blah not False" -> "blah is False"

Whichever way your team wants to interpret it, d00d.

Please consider whether you should be writing "(blah is False) is
True", that would be more explicit.
 
N

Neil Cerutti

Whichever way your team wants to interpret it, d00d.

Please consider whether you should be writing "(blah is False)
is True", that would be more explicit.

OK, now we're entering Daily WTF territory. ;)

And in the original case, I'd agree that "if X.has_key():" is
quite clear, already yielding a boolian value, and so doesn't
need to be tested for if it's False. But I wouldn't like to test
for an empty list or for None implicitly.
 
G

Gabriel Genellina

Yes, this violates the Holy, Inspired, Infallible Style Guide (pbuh),
which was written by the very finger of God when the world was still in

It's not about style or being pythonic: a condition may be false, but
not the False object itself, so writing
if something is False:
is the wrong way to test if something is false (isn't clear? :) )
.... else: print "3 is even"
....
3 is even


Gabriel Genellina
Softlab SRL





__________________________________________________
Preguntá. Respondé. Descubrí.
Todo lo que querías saber, y lo que ni imaginabas,
está en Yahoo! Respuestas (Beta).
¡Probalo ya!
http://www.yahoo.com.ar/respuestas
 
H

hanumizzle

The following line of lightly munged code was found in a publicly
available Python library...

if schema.elements.has_key(key) is False:

if not schema.elements.has_key(key): or, actually, if not key in
schema.elements: is how I would write it, but this reads more like
English.

-- Theerasak
 
M

MonkeeSage

And in the original case, I'd agree that "if X.has_key():" is
quite clear, already yielding a boolian value, and so doesn't
need to be tested for if it's False. But I wouldn't like to test
for an empty list or for None implicitly.

I agree that predicates are explicit in themselves, if they are named
intuitively like "has_key". I assumed that the OP was upset about "is
False" not that an explicit check was done on a predicate method.

It's not about style or being pythonic: a condition may be false, but
not the False object itself, so writing
if something is False:
is the wrong way to test if something is false (isn't clear? :) )

Yes, I understand, and it's true that in a mixed context it could bite
you (or you could check equality rather than identity), but assuming a
strictly boolean context you don't have that problem (or perhaps you
even *want* to differentiate between False, None and 0 in a mixed
context).

Ps. I mostly use the "if (not) cond" form. But I take the style guide
as a, well, *guide*, rather than a style *law* and I tend to get
annoyed when people get religious about things like that. Subjective
taste, be it GvR's or the Pope's, is -- subjective. On the other hand,
if lightening is flashing and seas are parting and stuff, I don;t tend
to argue too much in that case. ;)

Regards,
Jordan
 
V

Virgil Dupras

MonkeeSage said:
Yes, this violates the Holy, Inspired, Infallible Style Guide (pbuh),
which was written by the very finger of God when the world was still in
chaotic darkness. But I guess I'm an atheist when it comes to PEP 8. If
it is clearer to you to make the condition explicit ("blah not False"),
rather than implicit ("not blah"), then use the former. I say write the
code the way *you* (and your team if applicable) are best able to read,
write and maintain it. Then when other people tell you that it isn't
good style, or isn't "pythonic," just stab them in the face with
soldering iron ala Chris Walken. :)

Regards,
Jordan

I don't think it's even a matter of "If it's clearer to *you*", it's a
matter of not writing code while drunk. Isn't something like "key not
in schema.elements" universally clearer than the above mess? (If
elements has a "has_key()" method, it probably is or acts like a dict)

But I don't blame the autor of this line (The drunk thing was a joke),
I can see how it can happen: editing. You write something, and the
oops, it doesn't quite work, make a quick fix, then another, then
another. You end up with something that works, but don't notice the
ugly line you left. It happens.

However, it can *also* happen when you write code while drunk...
 
A

Aahz

Yes, this violates the Holy, Inspired, Infallible Style Guide (pbuh),
which was written by the very finger of God when the world was still in
chaotic darkness.

Did you actually analyze the line of code? Particularly WRT the way it
operates in different versions of Python?
 
J

John Machin

Aahz said:
Did you actually analyze the line of code? Particularly WRT the way it
operates in different versions of Python?

A comment on the "style" issue, before we get into the real WTF
analysis: any function/method whose name begins with "has" or "is"
returns an honest-to-goodness actual bool (or what passed for one in
former times). IMHO, any comparison with [] being regarded as false and
[0] being regarded as true is irrelevant, and writing "has_something()
== False" or "has_something() is False" is utterly ludicrous.

Now back to the analysis. Empirical evidence, back as far as 2.1:

C:\junk>type isfalse.py
import sys
print sys.version
try:
False
print "False is defined; type: %r; value: %r" \
% (type(False), False)
except NameError:
print "defining: False = 0; True = 1"
False = 0
True = 1
adict = {1: 42}
v = adict.has_key(2)
print "'adict.has_key(2)' ->", v
print "'adict.has_key(2) == False' ->", v == False
print "'adict.has_key(2) is False' ->", v is False
try:
1 in adict
print "'key in adict' is available"
except TypeError, e:
print "Need to use 'adict.has_key(key)'"
print "'key in adict' -> %s" % e


C:\junk>for %1 in (5 4 3 2 1) do \python2%1\python isfalse.py

C:\junk>\python25\python isfalse.py
2.5 (r25:51908, Sep 19 2006, 09:52:17) [MSC v.1310 32 bit (Intel)]
False is defined; type: <type 'bool'>; value: False
'adict.has_key(2)' -> False
'adict.has_key(2) == False' -> True
'adict.has_key(2) is False' -> True
'key in adict' is available

C:\junk>\python24\python isfalse.py
2.4.3 (#69, Mar 29 2006, 17:35:34) [MSC v.1310 32 bit (Intel)]
False is defined; type: <type 'bool'>; value: False
'adict.has_key(2)' -> False
'adict.has_key(2) == False' -> True
'adict.has_key(2) is False' -> True
'key in adict' is available

C:\junk>\python23\python isfalse.py
2.3.5 (#62, Feb 8 2005, 16:23:02) [MSC v.1200 32 bit (Intel)]
False is defined; type: <type 'bool'>; value: False
'adict.has_key(2)' -> False
'adict.has_key(2) == False' -> True
'adict.has_key(2) is False' -> True
'key in adict' is available

C:\junk>\python22\python isfalse.py
2.2.3 (#42, May 30 2003, 18:12:08) [MSC 32 bit (Intel)]
False is defined; type: <type 'int'>; value: 0
'adict.has_key(2)' -> 0
'adict.has_key(2) == False' -> 1
'adict.has_key(2) is False' -> 0
'key in adict' is available

C:\junk>\python21\python isfalse.py
2.1.3 (#35, Apr 8 2002, 17:47:50) [MSC 32 bit (Intel)]
defining: False = 0; True = 1
'adict.has_key(2)' -> 0
'adict.has_key(2) == False' -> 1
'adict.has_key(2) is False' -> 1
Need to use 'adict.has_key(key)'
'key in adict' -> 'in' or 'not in' needs sequence right argument

Analysis depends on what range of Python versions the author purported
to be supporting:
(1) back to 2.3: bad style, no damage, but why not use "key in dict"?
(2) back to 2.2: bad style, *WRONG ANSWER*; why not use "key in dict"?
(3) back to 2.1: as above for 2.2; would not work in 2.1 without the
demonstrated band-aid. Note that the simple "if not
adict.has_key(key):" construct works happily (if a trifle slowly) in
all 5 versions.

HTH,
John
 
N

Neil Cerutti

I agree that predicates are explicit in themselves, if they are
named intuitively like "has_key". I assumed that the OP was
upset about "is False" not that an explicit check was done on a
predicate method.

And that's something that I'd never have written, and wouldn't
have recognized as a bug until this thread. I can hear the gears
clicking in there now. Thanks to you and other that explained
this bug properly.
 
S

Steven D'Aprano

Whichever way your team wants to interpret it, d00d.

Please consider whether you should be writing "(blah is False) is
True", that would be more explicit.

Puh-lease! Get it right!

It should be "((blah is False) is True) is True".
 
J

John Machin

Steven said:
Puh-lease! Get it right!

It should be "((blah is False) is True) is True".

Yes, but it stops after one more iteration. "What I tell you three
times is true" -- the Bellman, "The Hunting of the Snark", by Lewis
Carroll.
 
J

John Roth

Aahz said:
The following line of lightly munged code was found in a publicly
available Python library...

if schema.elements.has_key(key) is False:

Sorry, just had to vent.

Uh, guys. IMO, the clearest way of writing this is:

if key not in schema.elements:
whatever...

Unless, of course, your code has to run in a
Python release earlier than 2.2. But then you
wouldn't be testing for the False object anyway.



John Roth
 
J

John Machin

John said:
Uh, guys. IMO, the clearest way of writing this is:

if key not in schema.elements:
whatever...

Uh, guys, what? You are the 3rd to make that point. Therefore it must
be true.
 
A

Aahz

Aahz said:
Did you actually analyze the line of code? Particularly WRT the way it
operates in different versions of Python?

A comment on the "style" issue, before we get into the real WTF
analysis: any function/method whose name begins with "has" or "is"
returns an honest-to-goodness actual bool (or what passed for one in
former times). IMHO, any comparison with [] being regarded as false and
[0] being regarded as true is irrelevant, and writing "has_something()
== False" or "has_something() is False" is utterly ludicrous.

Exactly. Another way of putting this: it's so wrong, it isn't even
wrong.
 
S

Scott David Daniels

John said:
... any function/method whose name begins with "has" or "is"
returns an honest-to-goodness actual bool (or what passed for
one in former times).

True for 75% of the builtins: if nm.startswith('is') or nm.startswith('has'))
['hasattr', 'hash', 'isinstance', 'issubclass']
>>> [hasattr(type,'mro'), hash(123), isinstance(type, type),
issubclass(type, type)]
[True, 123, True, True]

:)

--Scott David Daniels
(e-mail address removed)
 
J

John J. Lee

The following line of lightly munged code was found in a publicly
available Python library...

if schema.elements.has_key(key) is False:

Sorry, just had to vent.

I think I was reading the same code recently (epydoc?) and was also
momentarily horrified ;-) until I realized that it was quite
deliberately using three-valued logic (True, False, None) for some
presumably-sensible reason. Since None is false, they had to use
"is". So, given the need for three-valued logic, it's not as silly as
it looks.


John
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,432
Messages
2,571,682
Members
48,796
Latest member
Greg L.

Latest Threads

Top