Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.9] bpo-42073: allow classmethod to wrap other classmethod-like descriptors. #22757

Closed
wants to merge 1 commit into from

Conversation

eriknw
Copy link
Contributor

@eriknw eriknw 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):

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 #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__`.
@eriknw eriknw 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

@ambv ambv requested review from vsajip, warsaw and a team as code owners July 13, 2021 14:52
@ambv ambv changed the base branch from main to 3.9 July 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants