Skip to content

Conversation

@eriknw
Copy link
Contributor

@eriknweriknw commented Oct 18, 2020

bpo-19072 (#8405) allows classmethod to wrap other descriptors, but this does not work when the wrapped descriptor mimics classmethod. The current PR fixes this.

In Python 3.8 and before, one could create a callable descriptor such that this works as expected (see Lib/test/test_decorators.py for examples):

classA: @myclassmethoddeff1(cls): returncls@classmethod@myclassmethoddeff2(cls): returncls

In Python 3.8 and before, A.f2() return A. Currently in Python 3.9, it returns type(A). This PR make A.f2() return A again.

As of #8405, classmethod calls obj.__get__(type) if obj has __get__. This allows one to chain @classmethod and @property together. When using classmethod-like descriptors, it's the second argument to __get__--the owner or the type--that is important, but this argument is currently missing. Since it is None, the "owner" argument is assumed to be the type of the first argument, which, in this case, is wrong (we want A, not type(A)).

This PR updates classmethod to call obj.__get__(type, type) if obj has __get__.

This PR is targeting Python 3.9 branch, because I think this is a bug fix. I can also target master (3.10).

I'm not really sure where to add a note of this change.

https://bugs.python.org/issue42073

bpo-19072 (python#8405) allows `classmethod` to wrap other descriptors, but this does not work when the wrapped descriptor mimics classmethod. The current PR fixes this. In Python 3.8 and before, one could create a callable descriptor such that this works as expected (see Lib/test/test_decorators.py for examples): ```python class A: @myclassmethod def f1(cls): return cls @classmethod @myclassmethod def f2(cls): return cls ``` In Python 3.8 and before, `A.f2()` return `A`. Currently in Python 3.9, it returns `type(A)`. This PR make `A.f2()` return `A` again. As of python#8405, classmethod calls `obj.__get__(type)` if `obj` has `__get__`. This allows one to chain `@classmethod` and `@property` together. When using classmethod-like descriptors, it's the second argument to `__get__`--the owner or the type--that is important, but this argument is currently missing. Since it is None, the "owner" argument is assumed to be the type of the first argument, which, in this case, is wrong (we want `A`, not `type(A)`). This PR updates classmethod to call `obj.__get__(type, type)` if `obj` has `__get__`.
@eriknweriknw changed the title bpo-42073: allow classmethod to wrap other classmethod-like descriptors.[3.9] bpo-42073: allow classmethod to wrap other classmethod-like descriptors.Oct 18, 2020
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@ambvambv requested review from a team, vsajip and warsaw as code ownersJuly 13, 2021 14:52
@ambvambv changed the base branch from main to 3.9July 13, 2021 14:53
@ambv
Copy link
Contributor

ambv commented Jul 13, 2021

Oops, changing the base of the pull request didn't do what I thought it would and reverting back to 3.9 leaves the mess as is. I'll recreate this pull request on main from scratch.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviewstaleStale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@eriknw@ambv@the-knights-who-say-ni@bedevere-bot