[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset bbe3c57c865439f2194eb760a4362b5506d221a7 by Miss Islington (bot) 
in branch '3.10':
bpo-19072: Classmethod can wrap other classmethod like descriptors (GH-29634) 
(GH-29643)
https://github.com/python/cpython/commit/bbe3c57c865439f2194eb760a4362b5506d221a7


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[miss-islington]

Change by miss-islington :


--
pull_requests: +27875
pull_request: https://github.com/python/cpython/pull/29643

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset e34809e1c2a09478f4e0651d551c9c12d3c556ab by Raymond Hettinger in 
branch 'main':
bpo-19072: Classmethod can wrap other classmethod like descriptors (GH-29634)
https://github.com/python/cpython/commit/e34809e1c2a09478f4e0651d551c9c12d3c556ab


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Change by Raymond Hettinger :


--
pull_requests: +27866
pull_request: https://github.com/python/cpython/pull/29634

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset 2ce8af3cbcb368a35a05a5a9f97a09405124f239 by Miss Islington (bot) 
in branch '3.10':
bpo-42073: allow classmethod to wrap other classmethod-like descriptors 
(GH-27115) (GH-27162)
https://github.com/python/cpython/commit/2ce8af3cbcb368a35a05a5a9f97a09405124f239


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[miss-islington]

Change by miss-islington :


--
pull_requests: +25699
pull_request: https://github.com/python/cpython/pull/27162

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset b83861f0265e07207a6ae2c49c40fa8f447893f2 by Łukasz Langa in 
branch 'main':
bpo-42073: allow classmethod to wrap other classmethod-like descriptors (#27115)
https://github.com/python/cpython/commit/b83861f0265e07207a6ae2c49c40fa8f447893f2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Łukasz Langa]

Change by Łukasz Langa :


--
nosy: +lukasz.langa
nosy_count: 11.0 -> 12.0
pull_requests: +25663
pull_request: https://github.com/python/cpython/pull/27115

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Irit Katriel]

Irit Katriel  added the comment:

I've created a followup issue re documentation of this change: issue42832

--
nosy: +iritkatriel

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger  added the comment:


New changeset c17f63fae57dc02e78bd6931b8fb6c6c2f9d4d81 by Miss Skeleton (bot) 
in branch '3.9':
bpo-19072: Update descriptor howto for decorator chaining (GH-22934) (GH-22935)
https://github.com/python/cpython/commit/c17f63fae57dc02e78bd6931b8fb6c6c2f9d4d81


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger  added the comment:


New changeset 8e5b0fdce337ef0a1f4f38b31a8c6b66c56b16d2 by Raymond Hettinger in 
branch 'master':
bpo-19072: Update descriptor howto for decorator chaining (GH-22934)
https://github.com/python/cpython/commit/8e5b0fdce337ef0a1f4f38b31a8c6b66c56b16d2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 9.0 -> 10.0
pull_requests: +21855
pull_request: https://github.com/python/cpython/pull/22935

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Change by Raymond Hettinger :


--
pull_requests: +21854
pull_request: https://github.com/python/cpython/pull/22934

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Erik Welch]

Change by Erik Welch :


--
nosy: +eriknw
nosy_count: 8.0 -> 9.0
pull_requests: +21721
pull_request: https://github.com/python/cpython/pull/22757

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Change by Raymond Hettinger :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger  added the comment:


New changeset 805f8f9afea116c5d4d000570e3d02ae84502f43 by Raymond Hettinger 
(Berker Peksag) in branch 'master':
bpo-19072: Make @classmethod support chained decorators (GH-8405)
https://github.com/python/cpython/commit/805f8f9afea116c5d4d000570e3d02ae84502f43


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Berker Peksag]

Change by Berker Peksag :


--
pull_requests: +7931

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I think it is impossible to get significant impact on the Python benchmark 
suite from this patch. But mickrobenchmarks can expose the regression if it 
exists. Something like:

./python -m perf timeit -s 'class A:' -s ' @classmethod' -s ' def cm(cls): 
pass' -- 'A.cm()'
./python -m perf timeit -s 'class A:' -s ' @classmethod' -s ' def cm(cls): 
pass' -s 'f = A.cm' -- 'f()'

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Berker Peksag]

Berker Peksag  added the comment:

> Berker, do you mind to create a PR?

I will submit a PR tomorrow.

Do you have specific ideas for a micro-benchmark in mind or do you want to me 
just run the Python benchmark suite against the patch?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Berker, do you mind to create a PR?

Supporting class properties looks good rationale to me. But we need to check 
how this change affects performance.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
versions: +Python 3.8 -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

With the patch class properties work:

>>> class A:
... @classmethod
... @property
... def __doc__(cls):
... return 'A doc for %r' % cls.__name__
... 
>>> A.__doc__
"A doc for 'A'"

This is worth to be explicitly documented.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Berker Peksag]

Berker Peksag added the comment:

Here is an updated patch with a test (adapted from msg198274.)

--
nosy: +berker.peksag
stage:  -> patch review
versions: +Python 3.6 -Python 3.4
Added file: http://bugs.python.org/file42737/issue19072.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Ionel Cristian Mărieș]

Changes by Ionel Cristian Mărieș :


--
nosy: +ionelmc

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger added the comment:

Graham, do we have a contributor agreement from you?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Graham Dumpleton]

Graham Dumpleton added the comment:

I don't believe so.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Antoine Pitrou]

Antoine Pitrou added the comment:

Well... I've not written enough descriptor-implementing code to have a clear 
opinion on this, but this looks quite obscure. I have personally never needed 
anything like the wrapt library (I've also never used the PyPI decorator 
module, FWIW).

--
nosy: +gvanrossum

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Guido van Rossum]

Guido van Rossum added the comment:

@grahamd: I occasionally have felt the pain of wrapping @classmethod (or 
@staticmethod). Never enough though to think of how to fix it. I really don't 
have the stomach to review your wrapt library, but your code looks okay except 
for style and missing tests. I'd also recommend adding a few words to the docs. 
(And yes, all of this is your responsibility -- nobody has time to do all that 
stuff for you.)

Style-wise:
- the continuation line in your patch is not properly formatted;
- either the else block should also use { } or the else clause should be 
omitted.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger added the comment:

Antoine, do you have any thoughts on this proposal?

--
nosy: +pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger added the comment:

I'll take a look at this in more detail in the next week or so.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Graham Dumpleton]

Graham Dumpleton added the comment:

If you have the time, would be great if you can have a quick look at my wrapt 
package. That will give you an idea of where I am coming from in suggesting 
this change.

http://wrapt.readthedocs.org/en/latest/
http://wrapt.readthedocs.org/en/latest/issues.html
http://wrapt.readthedocs.org/en/latest/decorators.html
http://wrapt.readthedocs.org/en/latest/examples.html

In short, aiming to be able to write decorators which are properly transparent 
and aware of the context they are used in, so we don't have this silly 
situation at the moment where it is necessary to write distinct decorators for 
regular functions and instance methods. A classmethod around another decorator 
was the one place things will not work as would like to see them work.

I even did a talk about writing better decorators at PyCon NZ. Slides with 
notes at:

http://lanyrd.com/2013/kiwipycon/scpkbk/

Thanks.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Daniel Urban]

Changes by Daniel Urban urban.dani...@gmail.com:


--
nosy: +daniel.urban

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Changes by Raymond Hettinger raymond.hettin...@gmail.com:


--
assignee:  - rhettinger
nosy: +rhettinger

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Raymond Hettinger]

Raymond Hettinger added the comment:

I don't think it was ever intended that decorators be chained together.  

The whole point is to control binding behavior during dotted look-up (when 
__getattribute__ is called) and not in other circumstances (such as a direct 
lookup in a class dictionary).

Note that classmethods typically wrap regular functions which have both 
__call__ and __get__ methods.   The classmethod object intentionally invokes 
the former instead of the latter which would unhelpfully create an inner bound 
or unbound method.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Graham Dumpleton]

Graham Dumpleton added the comment:

The classmethod __get__() method does:

static PyObject *
cm_descr_get(PyObject *self, PyObject *obj, PyObject *type)
{
classmethod *cm = (classmethod *)self;

if (cm-cm_callable == NULL) {
PyErr_SetString(PyExc_RuntimeError,
uninitialized classmethod object);
return NULL;
}
if (type == NULL)
type = (PyObject *)(Py_TYPE(obj));
return PyMethod_New(cm-cm_callable,
type, (PyObject *)(Py_TYPE(type)));
}

So it isn't intentionally calling __call__(). If it still doing binding, but 
doing it by calling PyMethod_New() rather than using __get__() on the wrapped 
function. Where it wraps a regular function the result is same as if __get__() 
was called as __get__() for a regular function internally calls PyMethod_New() 
in the same way.

static PyObject *
func_descr_get(PyObject *func, PyObject *obj, PyObject *type)
{
if (obj == Py_None)
obj = NULL;
return PyMethod_New(func, obj, type);
}

By not using __get__(), you deny the ability to have chained decorators that 
want/need the knowledge of the fact that binding was being done. The result for 
stacking multiple decorators which use regular functions (closures) is exactly 
the same, but you open up other possibilities of smarter decorators.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
versions: +Python 3.4 -Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19072
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19072] classmethod doesn't honour descriptor protocol of wrapped callable

[Graham Dumpleton]

New submission from Graham Dumpleton:

The classmethod decorator when applied to a function of a class, does not 
honour the descriptor binding protocol for whatever it wraps. This means it 
will fail when applied around a function which has a decorator already applied 
to it and where that decorator expects that the descriptor binding protocol is 
executed in order to properly bind the function to the class.

A decorator may want to do this where it is implemented so as to be able to 
determine automatically the context it is used in. That is, one magic decorator 
that can work around functions, instance methods, class methods and classes, 
thereby avoiding the need to have multiple distinct decorator implementations 
for the different use case.

So in the following example code:

class BoundWrapper(object):
def __init__(self, wrapped):
self.__wrapped__ = wrapped
def __call__(self, *args, **kwargs):
print('BoundWrapper.__call__()', args, kwargs)
print('__wrapped__.__self__', self.__wrapped__.__self__)
return self.__wrapped__(*args, **kwargs)

class Wrapper(object):
def __init__(self, wrapped):
self.__wrapped__ = wrapped
def __get__(self, instance, owner):
bound_function = self.__wrapped__.__get__(instance, owner)
return BoundWrapper(bound_function)

def decorator(wrapped):
return Wrapper(wrapped)

class Class(object):
@decorator
def function_im(self):
print('Class.function_im()', self)

@decorator
@classmethod
def function_cm_inner(cls):
print('Class.function_cm_inner()', cls)

@classmethod
@decorator
def function_cm_outer(cls):
print('Class.function_cm_outer()', cls)

c = Class()

c.function_im()
print()
Class.function_cm_inner()
print()
Class.function_cm_outer()

A failure is encountered of:

$ python3.3 cmgettest.py
BoundWrapper.__call__() () {}
__wrapped__.__self__ __main__.Class object at 0x1029fc150
Class.function_im() __main__.Class object at 0x1029fc150

BoundWrapper.__call__() () {}
__wrapped__.__self__ class '__main__.Class'
Class.function_cm_inner() class '__main__.Class'

Traceback (most recent call last):
  File cmgettest.py, line 40, in module
Class.function_cm_outer()
TypeError: 'Wrapper' object is not callable

IOW, everything is fine when the decorator is applied around the classmethod, 
but when it is placed inside of the classmethod, a failure occurs because the 
decorator object is not callable.

One could argue that the error is easily avoided by adding a __call__() method 
to the Wrapper class, but that defeats the purpose of what is trying to be 
achieved in using this pattern. That is that one can within the bound wrapper 
after binding occurs, determine from the __self__ of the bound function, the 
fact that it was a class method. This can be inferred from the fact that 
__self__ is a class type.

If the classmethod decorator tp_descr_get implementation is changed so as to 
properly apply the descriptor binding protocol to the wrapped object, then what 
is being described is possible.

Having it honour the descriptor binding protocol also seems to make application 
of the Python object model more consistent.

A patch is attached which does exactly this.

The result for the above test after the patch is applied is:

BoundWrapper.__call__() () {}
__wrapped__.__self__ __main__.Class object at 0x10ad237d0
Class.function_im() __main__.Class object at 0x10ad237d0

BoundWrapper.__call__() () {}
__wrapped__.__self__ class '__main__.Class'
Class.function_cm_inner() class '__main__.Class'

BoundWrapper.__call__() () {}
__wrapped__.__self__ class '__main__.Class'
Class.function_cm_outer() class '__main__.Class'

That is, the decorator whether it is inside or outside now sees things in the 
same way.

If one also tests for calling of the classmethod via the instance:

print()
c.function_cm_inner()
print()
c.function_cm_outer()

Everything again also works out how want it:

BoundWrapper.__call__() () {}
__wrapped__.__self__ class '__main__.Class'
Class.function_cm_inner() class '__main__.Class'

BoundWrapper.__call__() () {}
__wrapped__.__self__ class '__main__.Class'
Class.function_cm_outer() class '__main__.Class'

FWIW, the shortcoming of classmethod not applying the descriptor binding 
protocol to the wrapped object, was found in writing a new object proxy and 
decorator library called 'wrapt'. This issue in the classmethod implementation 
is the one thing that has prevented wrapt having a system of writing decorators 
that can magically work out the context it is used in all the time. Would be 
nice to see it fixed. :-)

The wrapt library can be found at:

https://github.com/GrahamDumpleton/wrapt
http://wrapt.readthedocs.org

The limitation in the classmethod implementation is noted in the wrapt 
documentation at:

http://wrapt.readthedocs.org/en/v1.1.2/issues.html#classmethod-get

--
components: Interpreter Core
files: funcobject.c.diff