classification
Title: The replacement suggested for callable(x) in py3k is not equivalent
Type: behavior Stage:
Components: 2to3 (2.x to 3.x conversion tool), Documentation, Interpreter Core Versions: Python 3.0, Python 3.1, Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: benjamin.peterson Nosy List: Trundle, benjamin.peterson, exarkun, ezio.melotti, georg.brandl, joe.amenta, milko.krachounov, ncoghlan, pitrou
Priority: normal Keywords: patch

Created on 2009-09-27 14:23 by milko.krachounov, last changed 2009-12-28 20:56 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
new_callable_semantics.patch joe.amenta, 2009-12-24 19:52 isinstance(obj, collections.Callable)
new_callable_semantics_moretests.patch joe.amenta, 2009-12-25 17:32 Same as the last one, but with more test cases to demonstrate touch_import's semantics
Messages (22)
msg93171 - (view) Author: Milko Krachounov (milko.krachounov) Date: 2009-09-27 14:23
hasattr(x, '__call__') has been suggested as a replacement for
callable(x) in the documentation and in the warning when running
python2.6 with -3. It is also what 2to3 replaces it with. However, the
two are not equivalent.

1. I can add a __call__ attribute to my object with "obj.__call__ =
lambda x:x". That will not make the object callable, callable() on
Python 2.6 returns False, but hasattr(obj, '__call__') returns True.
2. I can implement a __getattr__ that returns something for every
possible attribute name. Again, this doesn't make the object callable,
nor does callable(obj) in Python 2.6 return True.

I think a closer replacement for "callable(x)" would be "'__call__' in
vars(type(x))".
msg93175 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2009-09-27 15:59
In the PEP3100 [1] there's written, in the "to be removed" list:
  "callable(): just use hasattr(x, '__call__') (?) [2] [done]"
and the note refers to a pdf [2] that instead says:
  "callable(): just call it, already"

If callable() was removed to encourage the EAFP [3] style, replacing "if
callable(x):" with "if hasattr(x, '__call__'):" is just a less-accurate
and less-readable way of doing the same (wrong) thing [4] (also note the
(?) next to the hasattr() expression).

This might be OK for automated tools like 2to3, but not for the doc and
the warning. Humans should probably replace the expression with a
try/except instead of using hasattr().
Even for 2to3 there are better and more accurate solutions though, like
the one proposed by the OP (not really readable) and isinstance(x,
collections.Callable).

(Note: what the OP said is correct for new-style classes, for old-style
classes callable() and hasattr() have the same result).

[1]: http://www.python.org/dev/peps/pep-3100/
[2]: Python Regrets:
http://www.python.org/doc/essays/ppt/regrets/PythonRegrets.pdf
[3]: http://docs.python.org/glossary.html#term-eafp
[4]: http://docs.python.org/glossary.html#term-lbyl
msg93178 - (view) Author: Milko Krachounov (milko.krachounov) Date: 2009-09-27 16:08
My suggestion is not only unreadable, but wrong. It's even less accurate
than hasattr(x, '__call__'), as it doesn't look in all the classes in
the MRO. Using isinstance(x, collections.Callable) should probably be
the correct replacement for 2to3 and situation where checking this is
really needed, as it does look for __call__ in the whole MRO.
msg93477 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-10-03 05:12
hasattr(type(x), "__call__") is technically a more valid replacement due
to the usual matter of metaclass confusion.

(Although putting special methods on instance objects is a recipe for
trouble in more ways than just this one).
msg93478 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-10-03 05:13
Although we should seriously consider properly exposing special method
lookup at the Python level...
msg93479 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2009-10-03 06:13
Benjamin already replaced hasattr(x, "__call__") with hasattr(type(x),
"__call__") in the Python 3.0 "What's New" in r75090 and r75094, but
this still doesn't match completely the behavior of callable():

>>> class Foo(object): pass
...
>>> foo = Foo()
>>> callable(foo)
False
>>> hasattr(type(foo), '__call__')
True
>>> foo()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'Foo' object is not callable

There are also other places where hasattr(x, "__call__") is still
suggested/used (e.g. PEP3100).
msg93488 - (view) Author: Andreas Stührk (Trundle) Date: 2009-10-03 10:25
As every type is an instance of `type`, every type also has a
`__call__` attribute which means ``hasattr(type(x), '__call__')`` is
always true. `callable()` checks whether `tp_call` is set on the type,
which cannot be done in Python directly.
msg94364 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-10-22 16:26
2to3 still uses hasattr(x, 'callable').
msg94373 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-10-23 00:02
2009/10/22 Georg Brandl <report@bugs.python.org>:
>
> Georg Brandl <georg@python.org> added the comment:
>
> 2to3 still uses hasattr(x, 'callable')

Do you have strong opinions about this? IMO, hasattr(x, '__call__') is
compatible enough.
msg94376 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-10-23 07:37
Not really, that was the last thing to get this issue closed.
msg95400 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-11-17 20:56
How about bringing callable back since there is no obvious replacement
for it?  It's valuable as a LYBL check in circumstances where an object
submitted far away from the place where it's invoked.  Such uses can't
easily be replaced with direct calls to follow the recommended EAFP style.
msg95403 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-17 21:15
I also think isinstance(x, collections.Callable) is the correct
replacement. Even though it might give a different answer on weird
corner cases, it is semantically what you are looking for.
(too bad it has a stupid module placement)
msg95416 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-11-18 10:16
Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> I also think isinstance(x, collections.Callable) is the correct
> replacement. Even though it might give a different answer on weird
> corner cases, it is semantically what you are looking for.
> (too bad it has a stupid module placement)

Yes, I was very surprised not to find it as abc.Callable...

Regards,
Nick.
msg96871 - (view) Author: Joe Amenta (joe.amenta) Date: 2009-12-24 19:52
One such weird corner case:

from collections import Callable
class Spam(object):
   def __call__(self):
      return self

can_of_spam = Spam()
print callable(can_of_spam) == isinstance(can_of_spam, Callable) # True
del Spam.__call__
can_of_spam.__call__ = lambda can: 'spam'
print callable(can_of_spam) == isinstance(can_of_spam, Callable) # False

Regardless, attached a patch for the new proposed semantics
msg96877 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-12-25 07:11
That isn't a semantic change, it is exactly the same semantics as
callable() in 2.x:

>>> class Spam(object):
...   def __call__(self): pass
...
>>> callable(Spam())
True
>>> del Spam.__call__
>>> callable(Spam())
False
msg96878 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-12-25 07:13
Just confirming that 2.x also correctly ignores instance attributes (as
it should, since it looks at the tp_call slot):

>>> odd = Spam()
>>> odd.__call__ = lambda: 'very odd'
>>> callable(odd)
False
msg96879 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-12-25 07:16
The patch however does not look correct - the import should be added as
a global import at the beginning of affected files rather than inline
with each callable check.
msg96880 - (view) Author: Andreas Stührk (Trundle) Date: 2009-12-25 11:28
What Joe Amenta stumbled across is that ABCMeta caches its subclass 
checks. If you call ``isinstance(spam, Callable)`` and after that delete 
`type(spam).__call__`, every other call of ``isinstance(spam, Callable)`` 
will still return True.
msg96882 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-12-25 13:25
Ah yes, I misread the example. Agreed that that is a change in behaviour
then - it is possible to clear the caches if absolutely necessary, but
doing so isn't particularly portable.
msg96883 - (view) Author: Joe Amenta (joe.amenta) Date: 2009-12-25 17:32
I believe that this patch works like you described...

Attached a patch with more test cases to show this.

(the [1:] parts are to make the test cases readable; they will still pass 
if all the leading newlines are removed from the triple-quoted strings and 
all [1:] are removed :-)
msg96885 - (view) Author: Joe Amenta (joe.amenta) Date: 2009-12-25 17:47
To elaborate on my last comment:

- touch_import looks for the required import binding in any scope, and it 
will add a global import if not found, otherwise it leaves it alone
- the import added does not have a newline prefix, so if the newlines were 
left in, (without [1:]) the tests would fail.
  However, I found that the test cases are easier to read if they all 
start on the same indentation level, so adding a newline for the reader 
but telling the parser to ignore it makes readable, correct test cases.
msg96967 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-12-28 20:56
Fixed in r77093.
History
Date User Action Args
2009-12-28 20:56:27benjamin.petersonsetstatus: open -> closed
resolution: accepted
messages: + msg96967
2009-12-25 17:47:39joe.amentasetmessages: + msg96885
2009-12-25 17:32:54joe.amentasetfiles: + new_callable_semantics_moretests.patch

messages: + msg96883
2009-12-25 13:25:23ncoghlansetmessages: + msg96882
2009-12-25 11:28:13Trundlesetmessages: + msg96880
2009-12-25 07:16:15ncoghlansetmessages: + msg96879
2009-12-25 07:13:49ncoghlansetmessages: + msg96878
2009-12-25 07:11:53ncoghlansetmessages: + msg96877
2009-12-24 19:52:21joe.amentasetfiles: + new_callable_semantics.patch

nosy: + joe.amenta
messages: + msg96871

keywords: + patch
2009-11-18 10:16:33ncoghlansetmessages: + msg95416
title: The replacement suggested for callable(x) in py3k is not equivalent -> The replacement suggested for callable(x) in py3k is not equivalent
2009-11-17 21:15:57pitrousetnosy: + pitrou
messages: + msg95403
2009-11-17 20:56:55exarkunsetnosy: + exarkun
messages: + msg95400
2009-10-23 07:37:07georg.brandlsetmessages: + msg94376
2009-10-23 00:02:48benjamin.petersonsetmessages: + msg94373
2009-10-22 16:26:18georg.brandlsetassignee: georg.brandl -> benjamin.peterson
messages: + msg94364
2009-10-03 10:25:30Trundlesetnosy: + Trundle
messages: + msg93488
2009-10-03 06:13:27ezio.melottisetnosy: + benjamin.peterson
messages: + msg93479
2009-10-03 05:13:14ncoghlansetmessages: + msg93478
2009-10-03 05:12:32ncoghlansetnosy: + ncoghlan
messages: + msg93477
2009-09-27 16:08:30milko.krachounovsetmessages: + msg93178
2009-09-27 15:59:26ezio.melottisetpriority: normal
nosy: + ezio.melotti
messages: + msg93175

2009-09-27 14:23:53milko.krachounovcreate