Ticket #71 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

Flawed check for "None" output in statement.py

Reported by: JonathanGriffitts Assigned to:
Priority: moderate Keywords:
Cc:

Description

statement.py line 130 checks for valid output from a statement with the line:

if args[0] == None:

This can cause an exception if the returned value is of a type that redefines the == operator. The solution is to use the recommended Python idiom:

if args[0] is None:

Footnote: the text "== None" occurs many places in the reinteract sources. This is a deprecated style. Guido von Rossum has commented on it in a PEP:

http://www.python.org/dev/peps/pep-0008/

In the section on "Programming recommendations", he says: - Comparisons to singletons like None should always be done with 'is' or 'is not', never the equality operators.

Change History

03/06/09 15:59:26 changed by JonathanGriffitts

Sorry, I forgot to provide a test case. Here's a somewhat contrived example: 

class tmp(object):
    def __init__(self,i):
        self.i = i
        
    def __eq__(self,a):
        return a.i == self.i

        
t = tmp(9)

t

This raises an exception after the last line, in reinteract/statement.py line 130, in do_output

03/19/09 08:38:57 changed by otaylor

I've put in a fix for this one place:

http://git.fishsoup.net/cgit/reinteract/commit/?id=db6ea5a0fce84bf2495653e85bfb86cb82f88348

Thanks for reporting the problem!

In general I would take a patch that fixed up all the == None, != None in Reinteract to improve the style. (Even if I think that when it matters, one is already doing something fishy... :-)

04/02/09 23:47:56 changed by otaylor

  • status changed from new to closed.
  • resolution set to fixed.

I've gone ahead and made the blanket is None/is not None change myself. http://www.reinteract.org/trac/ticket/77 was too much to take - xmlrpclib was trying to marshal eq as a remote procedure call.

http://git.fishsoup.net/cgit/reinteract/commit/?id=2b71670f3c756c7720d8c43bd062d24fdd84559b