Skip to content

property.__get__: overload to model class-access behavior #13769

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.

Sign up for GitHub

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

Merged
srittau merged 1 commit into from
Apr 2, 2025
Merged

property.__get__: overload to model class-access behavior #13769

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions stdlib/builtins.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,9 @@ class property:
def getter(self, fget: Callable[[Any], Any], /) -> property: ...
def setter(self, fset: Callable[[Any, Any], None], /) -> property: ...
def deleter(self, fdel: Callable[[Any], None], /) -> property: ...
@overload
def __get__(self, instance: None, owner: type, /) -> Self: ...

@srittau srittau Apr 2, 2025

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.

The second argument is optional and unused, we should reflect that:

Suggested change
def __get__(self, instance: None, owner: type, /) -> Self: ...
def __get__(self, instance: None, owner: Unused = None, /) -> Self: ...

(Unused might need to be imported from _typeshed. Also, we might need to silence mypy's overlap warning.)

@AlexWaygood AlexWaygood Apr 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The second argument is optional

I don't think that's true? If None is supplied for the first parameter, it's invalid to leave the second parameter unspecified:

>>> class Foo:
...     @property
...     def bar(self): return 42
...     
>>> b = Foo.__dict__["bar"]
>>> b.__get__(None)
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    b.__get__(None)
    ~~~~~~~~~^^^^^^
TypeError: __get__(None, None) is invalid

Only if I provide the second paramter is it valid to give None for the first parameter:

>>> b.__get__(None, Foo)
<property object at 0x102f3c860>

It's true that if you provide an instance as the first parameter, rather than None, then it's okay to omit the second parameter:

>>> b.__get__(Foo())
42

but that's already covered by the second overload immediately below?

@srittau srittau Apr 2, 2025 •
edited
Loading

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.

I don't think that's true? If None is supplied for the first parameter, it's invalid to leave the second parameter unspecified:

Which means that both the linked pseudo code is wrong and that the C implementation contradicts the specification, but it is what it is ...

@AlexWaygood AlexWaygood Apr 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I think @sharkdp would be the first to tell you that we've been having "a lot of fun" recently at red-knot trying to figure out which bits of documentation, specification and pseudo-code regarding the descriptor protocol are actually precise descriptions of the real runtime behaviour, and which are just approximations 😆

@sharkdp sharkdp Apr 2, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which means that both the linked pseudo code is wrong and that the C implementation contradicts the specification, but it is what it is ...

Right. There is some precedent for modeling __get__ this way in typeshed. For example, see types.FunctionType or builtins.classmethod:

typeshed/stdlib/types.pyi

Lines 111 to 114 in 977f300

@overload
def __get__(self, instance: None, owner: type, /) -> FunctionType: ...
@overload
def __get__(self, instance: object, owner: type | None = None, /) -> MethodType: ...

typeshed/stdlib/builtins.pyi

Lines 160 to 163 in 977f300

@overload
def __get__(self, instance: _T, owner: type[_T] | None = None, /) -> Callable[_P, _R_co]: ...
@overload
def __get__(self, instance: None, owner: type[_T], /) -> Callable[_P, _R_co]: ...

For the property descriptor, it's not particularly important, because the owner attribute is not used. But for some descriptors, it is essential that owner is passed in when instance = None. For example, classmethod.__get__ needs the owner argument to create a <bound method Owner.some_class_method of <class 'Owner'>.

@overload
def __get__(self, instance: Any, owner: type | None = None, /) -> Any: ...
def __set__(self, instance: Any, value: Any, /) -> None: ...
def __delete__(self, instance: Any, /) -> None: ...
Expand Down