Replace 'callable(foo)' with 'hasattr(foo, "__call__")'

[cvaliente]

This patch eliminates the DeprecationWarnings thrown when running the test suite with "python2.6 -3"

[antoine]

Please note there are corner cases where the two are not equivalent, but hopefully they don't really matter in the real world:

>>> class P(property):
...   def __get__(self, obj, cls):
...     1/0
... 
>>> class C(object):
...   __call__ = P()
... 
>>> c = C()
>>> callable(c)
True
>>> hasattr(c, "__call__")
False
>>> c.__call__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in __get__
ZeroDivisionError: integer division or modulo by zero

The reason is that hasattr() really calls getattr() and then discards the result / catches any exception, while callable() only examines the type's structure (callable() is lower level).

[cvaliente]

Replying to antoine:

The reason is that hasattr() really calls getattr() and then discards the result / catches any exception, while callable() only examines the type's structure (callable() is lower level).

I see. What about defining callable in twisted.python.compat3 like this:

$ python3
Python 3.1.1 (r311:74480, Oct 29 2009, 08:31:28) 
[GCC 4.2.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> def callable(obj):
...    try:
...        return getattr(obj, "__call__", None) is not None
...    except:
...        return True
...
>>> class P(property):
...     def __get__(self, obj, cls):
...         1/0
... 
>>> class C(object):
...     __call__ = P()
... 
>>> c = C()
>>> callable(c)
True

[antoine]

It's still not ideal:

>>> class C: pass
... 
>>> c = C()
>>> c.__call__ = lambda: print("__call__!")
>>> getattr(c, "__call__", None)
<function <lambda> at 0x7fb4e58b5b78>
>>> c()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'C' object is not callable

[antoine]

Ok, so Benjamin Peterson answered me the following on IRC, about the ideal replacement:

isinstance(x, collections.Callable), but I think hasattr(x, "__call__") is fine

[cvaliente]

Replying to antoine:

Ok, so Benjamin Peterson answered me the following on IRC, about the ideal replacement: isinstance(x, collections.Callable), but I think hasattr(x, "__call__") is fine

Since that option seems to be better, shall I implement callable() in twisted.python.compat3, together with some test cases like the samples you provided here, Antoine?

[glyph]

Shouldn't this just be in twisted.python.compat? Why a new compat3 module?

[cvaliente]

Replying to glyph:

Shouldn't this just be in twisted.python.compat? Why a new compat3 module?

According to the description of #2484, compatibility code for Python 3.x should go in twisted.python.compat3. Does that still hold, or should I just keep it simple and do this in twisted.python.compat:

try:
    callable = callable
except NameError:
    def callable(obj):
        # ...

[antoine]

Please note that, looking at the kind of purposes callable() is used for here, hasattr(XXX, "__call__") is probably good enough a replacement (and faster than the correct approach). But the final decision is not my call :-)

By the way, there's a typo in your patch in test_assertions: "__call_" instead of "__call__".

[cvaliente]

By the way, there's a typo in your patch in test_assertions: "__call_" instead of "__call__".

Oops, just fixed that . Thanks, Antoine!

[exarkun]

hasattr is a Python mis-function:

   >>> class X:
   ...     def __getattr__(self, name):
   ...             1 / 0
   ... 
   >>> hasattr(X(), 'foo')
   False
   >>> getattr(X(), 'foo', None)
   Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
     File "<stdin>", line 3, in __getattr__
   ZeroDivisionError: integer division or modulo by zero
   >>> 

I think the twisted.python.compat idea makes the most sense.

[cvaliente]

Replying to exarkun:

I think the twisted.python.compat idea makes the most sense.

Patch callable-in-twisted-compat.diff implements callable in twisted.python.compat, as suggested.

[exarkun]

Thanks for the updated patch cvaliente.

The terrible implementation of callable in this patch motivated me to complain on #python-dev. Someone there pointed out  http://bugs.python.org/issue7006 which makes it anything but clear what callable is actually supposed to be replaced with. I think that waiting for that issue to be resolved before changing anything in Twisted makes sense.

[jknight]

Many (most? I didn't count.) uses of callable in twisted seem to be in "assert" expressions. These seem superfluous and should probably just be removed. Even if something is callable, it might still fail to be called, and that error must be handled properly, in any case.

[exarkun]

True. Some of the places do it so that the error happens early, near the code which is bogusly passing a non-callable in, though. I think these make it easier for people doing something dumb to recognize their mistake, maybe.

[exarkun]

Additional news,  http://bugs.python.org/issue7624 (isinstance(foo, collections.Callable) is presently broken for classic classes).

[thijs]

Replying to exarkun:

Additional news,  http://bugs.python.org/issue7624 (isinstance(foo, collections.Callable) is presently broken for classic classes).

Which is fixed now in Python trunk.

[exarkun]

Python 3.2 is re-introducing the callable builtin:  http://bugs.python.org/issue10518