classmethod does not pass "type/owner" when invoking wrapped __get__ #86239

Activity

1. eriknw commented on Oct 18, 2020

   

   eriknwmannequin

   on Oct 18, 2020

   MannequinAuthor

   More actions

   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`."

   #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 bpo-19072 (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!
2. eriknw

   added

   3.9 (EOL)end of lifeend of life

   3.10 (EOL)end of lifeend of life

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Oct 18, 2020
3. ambv commented on Jul 15, 2021

   

   ambv

   on Jul 15, 2021

   Contributor

   More actions

   Thanks! ✨ 🍰 ✨
4. ambv

   closed this as completedon Jul 15, 2021
5. ambv commented on Jul 15, 2021

   

   ambv

   on Jul 15, 2021

   Contributor

   More actions

   New changeset b83861f by Łukasz Langa in branch 'main':
   bpo-42073: allow classmethod to wrap other classmethod-like descriptors (bpo-27115)
   b83861f
6. ambv

   added

   3.11only security fixesonly security fixes

   and removed

   3.9 (EOL)end of lifeend of life

   on Jul 15, 2021
7. ambv commented on Jul 15, 2021

   

   ambv

   on Jul 15, 2021

   Contributor

   More actions

   New changeset 2ce8af3 by Miss Islington (bot) in branch '3.10':
   bpo-42073: allow classmethod to wrap other classmethod-like descriptors (GH-27115) (GH-27162)
   2ce8af3
8. ezio-melotti

   transferred this issue fromon Apr 10, 2022