classification
Title: classmethod does not pass "type/owner" when invoking wrapped __get__
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, eriknw, lukasz.langa, miss-islington, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2020-10-18 20:22 by eriknw, last changed 2021-07-15 13:42 by lukasz.langa. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22757 closed eriknw, 2020-10-18 22:25
PR 27115 merged lukasz.langa, 2021-07-13 15:45
PR 27162 merged miss-islington, 2021-07-15 13:19
Messages (4)
msg378893 - (view) Author: Erik Welch (eriknw) * Date: 2020-10-18 20:22
The following is new to Python 3.9, and I consider the implementation incomplete.  I have code that works for Python 3.8 and before, but not for Python 3.9:

"Class methods can now wrap other :term:`descriptors <descriptor>` such as :func:`property`."

https://github.com/python/cpython/pull/8405
https://bugs.python.org/issue19072

As implemented, `classmethod` does not correctly wrap descriptors that mimic classmethod.  Previously, __get__of wrapped objects wasn't invoked by classmethod, so it was safe to have an object with both __call__ and __get__ behave like a classmethod.  Now, classmethod calling __get__ first gives incorrect results.

Here is a minimal example:
```
from types import MethodType


class myclassmethod:
    def __init__(self, func):
        self.func = func

    def __call__(self, cls):
        return self.func(cls)

    def __get__(self, instance, owner=None):
        if owner is None:
            owner = type(instance)
        return MethodType(self, owner)


class A:
    @myclassmethod
    def f1(cls):
        return cls

    @classmethod
    @myclassmethod
    def f2(cls):
        return cls


assert A.f1() is A
assert A.f2() is A  # <-- fails in 3.9, works in 3.8 and before
```
This pattern would typically be used to do something extra in __call__.

For the sake of discussion, let's call the two arguments to __get__ "instance" and "owner".  Typically, "instance" is an instance of "owner", or, equivalently, "owner" is the type of "instance".  If "owner" is None, it is generally assumed to be the type of "instance".

In bpo19072 (and gh8405), classmethod was changed to call `obj.__get__(owner)` if the wrapped object "obj" has __get__.  Notice that only the "instance" argument is provided.  Moreover, the type `owner` is passed as the "instance" argument.  This means that the "owner" argument (which is None) will be assumed to be the type of the "instance" argument, which is the type of the `owner` type.  This is wrong.  The "owner" argument should be `owner`.

I believe it would be better for classmethod to call `obj.__get__(owner, owner)` if "obj" has __get__.

This is kind of difficult to explain.  I will make a PR with more informative tests shortly.  Here is the simple diff to make the above example pass:
```
diff --git a/Objects/funcobject.c b/Objects/funcobject.c
index bd24f67b97..74f9167566 100644
--- a/Objects/funcobject.c
+++ b/Objects/funcobject.c
@@ -739,7 +739,7 @@ cm_descr_get(PyObject *self, PyObject *obj, PyObject *type)
         type = (PyObject *)(Py_TYPE(obj));
     if (Py_TYPE(cm->cm_callable)->tp_descr_get != NULL) {
         return Py_TYPE(cm->cm_callable)->tp_descr_get(cm->cm_callable, type,
-                                                      NULL);
+                                                      type);
     }
     return PyMethod_New(cm->cm_callable, type);
 }

```
Since I consider the new behavior to have introduced a regression, I think this change should be applied to both 3.9 and 3.10.

Cheers!
msg397538 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-15 10:53
Thanks! ✨ 🍰 ✨
msg397548 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-15 13:16
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
msg397552 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-15 13:42
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
History
Date User Action Args
2021-07-15 13:42:15lukasz.langasetmessages: + msg397552
2021-07-15 13:19:55lukasz.langasetversions: + Python 3.11, - Python 3.9
2021-07-15 13:19:06miss-islingtonsetnosy: + miss-islington

pull_requests: + pull_request25698
2021-07-15 13:16:28lukasz.langasetmessages: + msg397548
2021-07-15 10:53:50lukasz.langasetstatus: open -> closed
resolution: fixed
messages: + msg397538

stage: patch review -> resolved
2021-07-13 15:45:30lukasz.langasetnosy: + lukasz.langa
pull_requests: + pull_request25662
2020-10-18 22:25:59eriknwsetkeywords: + patch
stage: patch review
pull_requests: + pull_request21720
2020-10-18 20:22:44eriknwcreate