Repository navigation
Not all method descriptors are callable #64508
Description
| BPO | 20309 |
|---|---|
| Nosy | @warsaw, @arigo, @rhettinger, @ncoghlan, @pitrou, @vstinner, @larryhastings, @bitdancer, @ericsnowcurrently, @jdemeyer, @eryksun, @sruggier |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/rhettinger'
closed_at = <Date 2015-04-14.14:53:36.344>
created_at = <Date 2014-01-20.06:43:37.890>
labels = ['interpreter-core', 'type-bug', 'invalid']
title = 'Not all method descriptors are callable'
updated_at = <Date 2021-03-31.14:45:13.131>
user = 'https://github.com/larryhastings'bugs.python.org fields:
activity = <Date 2021-03-31.14:45:13.131>
actor = 'vstinner'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2015-04-14.14:53:36.344>
closer = 'ncoghlan'
components = ['Interpreter Core']
creation = <Date 2014-01-20.06:43:37.890>
creator = 'larry'
dependencies = []
files = ['33559', '34928', '34959', '38942', '47940']
hgrepos = []
issue_num = 20309
keywords = ['patch']
message_count = 16.0
messages = ['208524', '208525', '208531', '208537', '208538', '208544', '216593', '216676', '216769', '240686', '240843', '240894', '240898', '330227', '338473', '389908']
nosy_count = 15.0
nosy_names = ['barry', 'arigo', 'rhettinger', 'ncoghlan', 'pitrou', 'vstinner', 'chrish42', 'larry', 'Arfrever', 'r.david.murray', 'eric.snow', 'jdemeyer', 'eryksun', 'ozialien', 'sruggier']
pr_nums = []
priority = 'low'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue20309'
versions = ['Python 3.5']
Activity
larryhastings commentedon Jan 20, 2014
I found something curious while experimenting with types tonight.
Everybody knows, if you access a descriptor in a class through the normal attribute-getting methods, you actually get the result of calling its '__get__' method. If you want to get your hands on the descriptor itself, you can be sneaky and pull it out through the class's __dict__.
I did an experiment tonight, accessing a bunch of methods in a bunch of different ways.
The methods I tried were
I accessed them
And for each of the above, I tried
That's a total of eighteen methods tried (3x3x2).
For each one, I printed out whether or not it was callable. I found the results startling: fifteen were, but three (!) were not.
All three that weren't were accessed through the class dict. They were:
I find it strange that the static method descriptors aren't callable.
I find it *even stranger* that *one* of the class method descriptors isn't callable, but the other one is.
I guess a foolish consistency is the hobgoblin of my small mind, but... shouldn't all eighteen of these objects be callable?
Attached is the test harness I wrote.
(p.s. I attached you guys as nosy because I thought you might be interested.)
larryhastings commentedon Jan 20, 2014
I should add, I see the same results with current trunk and with my handy Python 3 (currently 3.3.1rc1, huh guess I'm behind).
[-]Class method descriptors are different between builtin and user classes[/-][+]Not all descriptors are callable[/+]arigo commentedon Jan 20, 2014
Instances of 'staticmethod' or 'classmethod' are not callable. I'm unsure why not, but it's good enough, as you need to go through various unexpected hops in order to try to call one.
'dict.fromkeys' is not a classmethod, as seen by "dict.__dict__['fromkeys']". It's an internal object usually never seen by Python code. I believe this is only because implementing a regular classmethod in C would be rather messy.
ncoghlan commentedon Jan 20, 2014
I believe it's just a matter of pattern of use - applying staticmethod
outside a class (or retrieving the descriptor directly from the dict,
bypassing the descriptor protocol), so nobody every noticed.
Ditto for the C wrapper vs the Python wrapper for a classmethod - it's
really rare to access those without going through the descriptor
machinery, so it's likely just an accident of implementation that one
of them isn't callable.
I don't see any specific reason for them to be non-callable - I
suspect it's just a case of never adding the code to make it happen.
[-]Not all descriptors are callable[/-][+]Class method descriptors are different between builtin and user classes[/+]ncoghlan commentedon Jan 20, 2014
Oh, so that's how that happens :)
(the title reverted because I replied via email from a part of the thread before Larry changed the issue name)
[-]Class method descriptors are different between builtin and user classes[/-][+]Not all method descriptors are callable[/+]bitdancer commentedon Jan 20, 2014
See also bpo-19072 for previous report of the classmethod problem.
chrish42 commentedon Apr 16, 2014
Work in progress for fixing this bug. (See descr_v1.diff) I converted the "curious.py" file into additional testcases. I started writing the functions for the tp_call slots for class and static methods.
To do: add tests to make sure that the code works for more than what's accepted by function_call(), then switch to using PyObject_Call() (which is the right function to use here, thanks to ncoghlan for catching that). Will finish this in the next few days.
eryksun commentedon Apr 17, 2014
classmethod_descriptor instances such as vars(dict)['fromkeys'] are callable. The first argument has to be a subclass of __objclass__:
>>> vars(dict)['fromkeys'].__objclass__ <class 'dict'>Calling the descriptor creates a bound built-in method; slices the args to remove the class; and calls it with the args and kwds.
>>> vars(dict)['fromkeys'](dict, 'abc') {'a': None, 'b': None, 'c': None}source: classmethoddescr_call
http://hg.python.org/cpython/file/04f714765c13/Objects/descrobject.c#l256
While the classmethod and staticmethod types that are defined in funcobject.c aren't callable, they do expose a __func__ member.
chrish42 commentedon Apr 18, 2014
Here is the (first?) complete version of the patch. All tests pass. Note that I had to remove a test that was checking that the object returned from staticmethod was not callable.
Let me know if I should add more tests, or if there are any other problems with the patch. Thanks!
chrish42 commentedon Apr 13, 2015
Updated patch to work with current state of repository. Tests still pass (including the new ones).
rhettinger commentedon Apr 14, 2015
I don't agree that this is a bug that should be fixed. It adds code that will likely never get called or needed (i.e. there has never been a request for this in the decade long history of desciptors and it seems like a made up requirement to me.
I concur with Amrin's comment that this "is good enough" and Nick's comment that "it's just a matter of pattern of use'. I recommend that this be closed as "not-a-bug" and that we avoid gratuitous API expansion for what Larry states is likely "foolish consistency".
If someone object to recommendation to close and really wants to push for this, I recommend making a business case for acceptance and then assigning this issue to Guido for a decision. This is his code and AFAICT he intentionally didn't go down a number of possible paths for descriptors simply because there weren't motivating use cases. To the extent that __call__ was omitted on some descriptors, I don't think it was an accident (most of this code was developed as the same time by the same person with deliberate differences between each of them). His write-ups for descriptors when they were introduced make no mention of an implied requirement for callability.
FWIW, bpo-19072 isn't really related to this one. That issue is a feature request to support descriptor chainging and it isn't clear whether that should be supported or left as-in.
chrish42 commentedon Apr 14, 2015
As a newbie to the CPython source code (and as someone who started working on this bug because it was on the lists of easy bugs for PyCon 2014), I don't have a strong attachment either way, as long as some kind of decision is reached, and I can check this off my list.
If forced to take a stance, I would probably agree that this might be reaching into "foolish consistency" territory, as I just don't see myself ever using the new possibilities that this added code would allow.
If the decision is made to fix this, I'll improve the tests to actually call these new callables (and check that the result of the call is correct). I'll wait until the "we should fix this" decision is made to work on the patch again, though. But if this is closed as not-a-bug, I'll be a happy camper too (as I've learned some stuff about CPython internals in the process).
ncoghlan commentedon Apr 14, 2015
I agree with Raymond's recommendation - actually supporting this would mean adding code that would need to be maintained indefinitely without providing a compensating practical benefit, so I'm flagging this as "not a bug". Thanks Christian for nudging us to make a decision one way or the other.
If another implementation requests clarification, we might want to document that "directly callable-or-not" for these descriptors is formally an interpreter implementation detail, but in the absence of such a request, I think this issue serves as an adequate reference.
sruggier commentedon Nov 22, 2018
I hit this problem today with what I'd consider a valid use case: I wanted to use a static method as a default argument to a function on the same class. Within the class definition context, automatic unwrapping of the staticmethod object doesn't occur, so the default value ends up not being callable unless the staticmethod object is manually, explicitly unwrapped via __func__.
For those who prefer code to prose, I've attached example Python 2/3 code demonstrating what I mean.
The workaround is pretty easy, but on the other hand, it also looks like it would also be pretty straightforward to facilitate this use case without forcing the author to learn about this distinction and manually unwrap the static method, and I think it makes sense to do that.
jdemeyer commentedon Mar 20, 2019
See also PEP-579 (issue 11) and the thread https://mail.python.org/pipermail/python-ideas/2018-June/051572.html
vstinner commentedon Mar 31, 2021
I proposed again a similar idea, but only for @staticmethod: bpo-43682.