Skip to content

Make builtins.property generic #985

Closed
Closed
Make builtins.property generic#985
Labels
topic: featureDiscussions about new features for Python's type annotations

Description

@sobolevn
sobolevn
opened on Dec 19, 2021

Hi!

Here's how property type is defined right now from a typing perspective:

from typing import Any, Callable

class property(object):
    fget: Callable[[Any], Any] | None
    fset: Callable[[Any, Any], None] | None
    fdel: Callable[[Any], None] | None
    def __init__(
        self,
        fget: Callable[[Any], Any] | None = ...,
        fset: Callable[[Any, Any], None] | None = ...,
        fdel: Callable[[Any], None] | None = ...,
        doc: str | None = ...,
    ) -> None: ...
    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: ...
    def __get__(self, __obj: Any, __type: type | None = ...) -> Any: ...
    def __set__(self, __obj: Any, __value: Any) -> None: ...
    def __delete__(self, __obj: Any) -> None: ...

Source: https://github.com/python/typeshed/blob/12b79f64d7241024447eae8ca5d52779cca94ee7/stdlib/builtins.pyi#L947

Note, that it is not generic. But, this is a type that surely can be modeled as a generic type.

Motivation

Why? Because @property works with two major scenarios: getting some inner type and setting some extrenal type. This can be represented as property[GetType, SetType].

It might not seem very important to end users, because of how @propertys are used, but it quite importatnt for type-checkers and type stubs.

Here are some problems that we have in mypy with properties:

Yes, we can work around the fact we cannot simply express property[GetType, SetType], but I don't think that missing this important detail is good for the typing system in the long run.
Without this change we cannot treat @property as a regular descriptor, which follows the same rules.

Reference implementation

Implementing generic @property will require several changes.

CPython

I guess we would need something similar as we did in https://www.python.org/dev/peps/pep-0585/

Something like adding:

{"__class_getitem__", Py_GenericAlias, METH_O\|METH_CLASS, PyDoc_STR("See PEP 585")},

Here: https://github.com/python/cpython/blob/c8749b578324ad4089c8d014d9136bc42b065343/Objects/descrobject.c#L1567-L1568

typeshed

I am sure that this code can be improved, but it does its job as a reference / demo:

from typing import Any, Callable, Generic

G = TypeVar('G')  # think about variance
S = TypeVar('S')

class property(Generic[G, S]):
    fget: Callable[[Any], G] | None
    fset: Callable[[Any, S], None] | None
    fdel: Callable[[Any], None] | None
    def __init__(
        self,
        fget: Callable[[Any], G] | None = ...,
        fset: Callable[[Any, S], None] | None = ...,
        fdel: Callable[[Any], None] | None = ...,
        doc: str | None = ...,
    ) -> None: ...
    def getter(self, __fget: Callable[[Any], G]) -> property: ...
    def setter(self, __fset: Callable[[Any, S], None]) -> property: ...
    def deleter(self, __fdel: Callable[[Any], None]) -> property: ...
    def __get__(self, __obj: Any, __type: type | None = ...) -> Any: ...
    def __set__(self, __obj: Any, __value: Any) -> None: ...
    def __delete__(self, __obj: Any) -> None: ...

Playground: https://mypy-play.net/?mypy=latest&python=3.10&flags=strict%2Cdisallow-subclassing-any%2Cdisallow-untyped-calls%2Cdisallow-untyped-decorators&gist=e7cec4565d416a0981d4a65e619f1be6

It almost works as-is, but @property.setter part requires some extra handling from type-checkers.

Backwards compatibility

As far as I understand CPython's development process, we can add this new feature to 3.11 only.
I think that using from __future__ import annotations should also work for @property the same way PEP585 defines.

Next steps

If others agree with me that property[GetType, SetType] is a good thing to have, I can:

Looking forward to your feedback! 😊

Related issues

Refs:

Activity

  1. sobolevn
    added
    topic: featureDiscussions about new features for Python's type annotations
    on Dec 19, 2021

  2. AlexWaygood commented on Dec 19, 2021

    @AlexWaygood
    AlexWaygood
    on Dec 19, 2021
    Member

    See also:

  3. hauntsaninja commented on Dec 19, 2021

    @hauntsaninja
    hauntsaninja
    on Dec 19, 2021
    Collaborator

    (In particular, see erictraut's comments on the second link)

  4. sobolevn commented on Dec 19, 2021

    @sobolevn
    sobolevn
    on Dec 19, 2021
    MemberAuthor

    To address some of @erictraut's feedback about @properties in python/typeshed#5987

    Type checkers already implement many special cases for properties.

    Yes, this is my main motivation when I suggest adding generic properties support.
    At least in mypy we can remove quite a lot of special cases and make property a first-class citizen.

    The big challenge with making property generic is that there's five pieces of information that need to be parameterized

    1. Whether there's a getter
    2. The type that the getter returns
    3. Whether there's a setter
    4. The type that the setter accepts
    5. Whether there's a deleter

    This can easily be addressed:

    • property[int, NoReturn] is a read-only property
    • property[NoReturn, int] is a write-only property
    • I think that we can safely assume that __delete__ can be special cased in type-checkers, because as Eric said, no one actually uses it. And there's no concrete type information required
    • And as I said, we would have to special case @.setter as well (mutability is not representable in Python's type system)

    I also don't see how this change can be made with the existing property without generating a lot of new type errors in existing code bases.

    From my work on decorated property support in mypy, we can see that there are not so many errors in mypy_primer output. See related comment: python/mypy#11719 (comment)

  5. ariebovenberg commented on Dec 20, 2021

    @ariebovenberg
    ariebovenberg
    on Dec 20, 2021

    Another big opportunity of generic property could be easier annotation of read-only attributes.

    class A:
    foo: int  # read- and writable attribute
    bar: property[int]  # read-only attribute

# instead of
class A:
    foo: int

    @property
    def bar(self) -> int:
        ...

    This would be a big win for type stubs, as well as Protocols. A quick look at typeshed shows >10% of defs there are used simply for properties. A more readable representation would be a big win!

    See also: #922 (comment)

    also: since the get-only case is so common, it'd probably be logical to allow property with one type argument (where the second defaults to NoReturn). This would be consistent with how property on its own creates only a getter by default.

  6. gvanrossum commented on Dec 20, 2021

    @gvanrossum
    gvanrossum
    on Dec 20, 2021
    Member

    It's clear there is something valuable here; there seem to be no lack of other issues/PRs where this has been discussed before, so it's clear there's a need. I hope you all solve the puzzle!

    IIUC it should be possible to completely remove the special-casing for properties from type checkers, instead deferring completely to the more general descriptor semantics, right? Then we'd still have to debate some finer points about descriptors, but at least the behavior of properties would be fully defined by its definition in typeshed plus descriptor semantics.

  7. JelleZijlstra commented on Dec 20, 2021

    @JelleZijlstra
    JelleZijlstra
    on Dec 20, 2021
    Member

    it should be possible to completely remove the special-casing for properties

    The biggest blocker for that may be the @property.setter mechanic—I don't think that can be mapped to the current type system.

    (And the same for @property.deleter, if anyone uses that.)

  8. sobolevn commented on Dec 20, 2021

    @sobolevn
    sobolevn
    on Dec 20, 2021
    MemberAuthor

    IIUC it should be possible to completely remove the special-casing for properties from type checkers, instead deferring completely to the more general descriptor semantics, right?

    Yes, with some minor special-case points. Like identifying writable / read-only properties by @.setter definition / its absence. And the same with @.deleter.

    But, it is specific to property.setter and property.deleter which are not a part of a descriptor protocol.

    While x = property(get_x, set_x) would be completely typed with this proposal.

  9. gvanrossum commented on Dec 20, 2021

    @gvanrossum
    gvanrossum
    on Dec 20, 2021
    Member

    Hm, yeah, we'll need to special-case .setter and .deleter. Fine, as long it ends up being fully typed then. (I'm guessing one issue is that ordinarily you cannot override a method.)

  10. sobolevn commented on Dec 26, 2021

    @sobolevn
    sobolevn
    on Dec 26, 2021
    MemberAuthor

    CC @erictraut and @rchen152 from pyright and pytype 🙂

  11. erictraut commented on Dec 29, 2021

    @erictraut
    erictraut
    on Dec 29, 2021
    Collaborator

    I played around with this. Here's a variant of the proposed class definition:

    _G1 = TypeVar("_G1")
_G2 = TypeVar("_G2")
_S1 = TypeVar("_S1")
_S2 = TypeVar("_S2")

class property(Generic[_G1, _S1]):
    fget: Callable[[Any], _G1] | None
    fset: Callable[[Any, _S1], None] | None
    fdel: Callable[[Any], None] | None
    @overload
    def __new__(
        cls,
        fget: Callable[[Any], _G2],
        fset: None = ...,
        fdel: Callable[[Any], None] | None = ...,
        doc: str | None = ...,
    ) -> property[_G2, NoReturn]: ...
    @overload
    def __new__(
        cls,
        fget: Callable[[Any], _G2],
        fset: Callable[[Any, _S2], None],
        fdel: Callable[[Any], None] | None = ...,
        doc: str | None = ...,
    ) -> property[_G2, _S2]: ...
    def getter(self, __fget: Callable[[Any], _G2]) -> property[_G2, _S1]: ...
    def setter(self, __fset: Callable[[Any, _S2], None]) -> property[_G1, _S2]: ...
    def deleter(self, __fdel: Callable[[Any], None]) -> property[_G1, _S1]: ...
    def __get__(self, __obj: Any, __type: type | None = ...) -> _G1: ...
    def __set__(self, __obj: Any, __value: _S1) -> None: ...
    def __delete__(self, __obj: Any) -> None: ...

    Some observations:

    1. This change does allow a bunch of custom logic to be removed from pyright that is currently in place to handle properties. However, when this logic is removed, error messages for properties become a bit more opaque and harder to follow.
    2. Removing the special-casing eliminates proper type checking for deleters. Probably not a big deal.
    3. This is a breaking change. I don't see a way to make the new property class declaration work with type checkers that were designed to work with the old one and vice versa.

    Overall, I'm pretty negative on this change.

    It would have been nice if property were defined as generic earlier, but making the change now would be very disruptive, and it would be a step backward in some respects if type checkers removed all of their custom logic and error messages specifically for properties.

  12. sobolevn commented on Jan 11, 2022

    @sobolevn
    sobolevn
    on Jan 11, 2022
    MemberAuthor

    Thanks everyone for their feedback!

    Since no one is really interested in this, I am going to close this for now.
    Maybe later we can revisit this decision.

  13. sobolevn
    closed this as completedon Jan 11, 2022

  14. ofek commented on Dec 17, 2022

    @ofek
    ofek
    on Dec 17, 2022
    Sponsor

    @sobolevn Hmm, what gave you the impression that no one is interested in this? Many are it seems, including at my workplace.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    topic: featureDiscussions about new features for Python's type annotations

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions