Skip to content

Fix __set__ to __get__ in messages#13913

Merged
hauntsaninja merged 1 commit into
python:masterfrom
ikonst:patch-1
Oct 17, 2022
Merged

Fix __set__ to __get__ in messages#13913
hauntsaninja merged 1 commit into
python:masterfrom
ikonst:patch-1

Conversation

@ikonst

@ikonst ikonst commented Oct 17, 2022

Copy link
Copy Markdown
Contributor

We're dealing with the get descriptor here but calling it __set__, probably a copy-paste from similar code elsewhere. I think this only affects messages.

@ikonst

ikonst commented Oct 17, 2022

Copy link
Copy Markdown
Contributor Author

@sobolevn 👁️

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@hauntsaninja hauntsaninja left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@hauntsaninja hauntsaninja left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you encountered this in real code, want to add your test case?

@ikonst

ikonst commented Oct 17, 2022

Copy link
Copy Markdown
Contributor Author

If you encountered this in real code, want to add your test case?

TBH, I stumbled on it while reading the code and it threw me off :)

An attempt to reproduce would be contrived:

from typing import Any, TypeVar

T = TypeVar('T')


def decorator(f: T) -> T:
    ...


class Descriptor:
    @decorator
    def __get__(self, *args: Any, **kwargs: Any) -> int:
        ...


class Cls:
    value: Descriptor


var = Cls()
_ = var.value

In this case, plugin get_attribute_hook would be called with "Descriptor.__set__" ... but only if __get__ is decorated 😵.

Without the decorator, get_attribute_hook is called for "Cls.value" but not for "Descriptor.__get__". Is that correct, or perhaps the more meaningful issue?

@hauntsaninja
hauntsaninja merged commit 874a28f into python:master Oct 17, 2022
@ikonst
ikonst deleted the patch-1 branch October 17, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants