Pyright behavior with generic descriptors and protocols #5915

CaselIT · 2023-09-08T20:50:09Z

CaselIT
Sep 8, 2023

Hi,

I've noticed a difference between how mypy and pyright handles descriptors when using protocols.
I'm creating a discussion since I'm not sure if mypy or pyright has the correct behavior.

I noticed this in sqlalchemy, in the example posted in this issue sqlalchemy/sqlalchemy#10288.

Let's start with a setup of a descriptor that's equivalent to the one used by sqlalchemy

from typing import Any, Generic, TypeVar, overload

_T_co = TypeVar("_T_co", covariant=True)

class InstrumentedAttribute(Generic[_T_co]):
    '''The real version has a lot of methods'''
    def something(self) -> _T_co:
        ...

class Mapped(Generic[_T_co]):
    @overload
    def __get__(
        self, instance: None, owner: Any
    ) -> InstrumentedAttribute[_T_co]:
        ...

    @overload
    def __get__(self, instance: object, owner: Any) -> _T_co:
        ...

    def __get__(
        self, instance: object | None, owner: Any
    ) -> InstrumentedAttribute[_T_co] | _T_co:
        ...

Let's also define a couple of classes and a protocol

from typing import Protocol

class ParentProtocol(Protocol):
    name: Mapped[str]

class Parent:
    name: Mapped[str]

class Item:
    id: Mapped[int]
    parent: Mapped[Parent]

First question

def check_parent_protocol(p: ParentProtocol) -> str:
    return p.name


def check_parent(p: Parent) -> str:
    return p.name


check_parent(Parent) # error
check_parent(Parent())  # ok
check_parent_protocol(Parent) # ???
check_parent_protocol(Parent())  # ok

pyright reports the following

test1.py:57:14 - error: Argument of type "type[Parent]" cannot be assigned to parameter "p" of type "Parent" in function "check_parent"
   "type[type]" is incompatible with "type[Parent]" (reportGeneralTypeIssues)

while mypy reports

test1.py:57: error: Argument 1 to "check_parent" has incompatible type "type[Parent]"; expected "Parent"  [arg-type]
test1.py:59: error: Argument 1 to "check_parent_protocol" has incompatible type "type[Parent]"; expected "ParentProtocol"  [arg-type]
test1.py:59: note: Only class variables allowed for class object access on protocols, name is an instance variable of "Parent"

It seems that pyright has a different behavior when using a protocol versus a class. I'm not sure what the correct behaviour is in this case, but different behaviour seems strange.

Second question

class PropProtocol(Protocol):
    @property
    def parent(self) -> Mapped[ParentProtocol]:
        ...


def get_parent_name(child: PropProtocol) -> str:
    reveal_type(child.parent)
    return child.parent.name


get_parent_name(Item)
get_parent_name(Item())

pyright reports the following

test1.py:70:17 - information: Type of "child.parent" is "Mapped[ParentProtocol]"
test1.py:71:25 - error: Cannot access member "name" for type "Mapped[ParentProtocol]"
  Member "name" is unknown (reportGeneralTypeIssues)

while mypy does this

test1.py:70: note: Revealed type is "test1.ParentProtocol"
test1.py:74: error: Argument 1 to "get_parent_name" has incompatible type "type[Item]"; expected "PropProtocol"  [arg-type]
test1.py:74: note: Only class variables allowed for class object access on protocols, parent is an instance variable of "Item"

Here the difference are two:

as before pyright allows the type in get_parent_name(Item) since the function is typed with a protocol. Same consideration as above
pyright interprets the property as returning a descriptor, so it considers child.parent a descriptor instance, while mypy uses the gets defined in the descriptor.
Not sure what's the correct behaviour here, but since pyright accepts Item() as a PropProtocol it seems that the correct behaviour should be the mypy one

Third question

class AttrProtocol(Protocol):
    parent: Mapped[ParentProtocol]


def get_parent_name_again(child: AttrProtocol) -> str:
    return child.parent.name


get_parent_name_again(Item)
get_parent_name_again(Item())

pyright reports the following

test1.py:86:23 - error: Argument of type "type[Item]" cannot be assigned to parameter "child" of type "AttrProtocol" in function "get_parent_name_again"
  "parent" is invariant because it is mutable
  "parent" is an incompatible type
    "Mapped[Parent]" is incompatible with "Mapped[ParentProtocol]"
      Type parameter "_T_co@Mapped" is covariant, but "Parent" is not a subtype of "ParentProtocol"
        "Parent" is incompatible with protocol "ParentProtocol" (reportGeneralTypeIssues)
test1.py:87:23 - error: Argument of type "Item" cannot be assigned to parameter "child" of type "AttrProtocol" in function "get_parent_name_again"
  "Item" is incompatible with protocol "AttrProtocol"
    "parent" is invariant because it is mutable
    "parent" is an incompatible type
      "Mapped[Parent]" is incompatible with "Mapped[ParentProtocol]"
        Type parameter "_T_co@Mapped" is covariant, but "Parent" is not a subtype of "ParentProtocol"
          "Parent" is incompatible with protocol "ParentProtocol" (reportGeneralTypeIssues)

mypy instead reports

test1.py:86: error: Argument 1 to "get_parent_name_again" has incompatible type "type[Item]"; expected "AttrProtocol"  [arg-type]
test1.py:86: note: Following member(s) of "Item" have conflicts:
test1.py:86: note:     parent: expected "Mapped[ParentProtocol]", got "Mapped[Parent]"
test1.py:86: note: Only class variables allowed for class object access on protocols, parent is an instance variable of "Item"
test1.py:87: error: Argument 1 to "get_parent_name_again" has incompatible type "Item"; expected "AttrProtocol"  [arg-type]
test1.py:87: note: Following member(s) of "Item" have conflicts:
test1.py:87: note:     parent: expected "Mapped[ParentProtocol]", got "Mapped[Parent]"

Here there seems to a bug in pyright since it reports Type parameter "_T_co@Mapped" is covariant, but "Parent" is not a subtype of "ParentProtocol" but previously it was fine in check_parent

Otherwise I'm not sure if AttrProtocol should be preferred over PropProtocol. It seems to me that it should be the least ambiguous way of defining a protocol, but since nighter mypy nor pyright likes it suggests that it's not.

Sorry for the long post, but I kept finding differences in this case. Thanks for reading

Complete file to reproduce

# mypy: disable-error-code="empty-body"
from typing import Any, Generic, TypeVar, overload

_T_co = TypeVar("_T_co", covariant=True)


class InstrumentedAttribute(Generic[_T_co]):
    def something(self) -> _T_co:
        ...

    # many other methods


class Mapped(Generic[_T_co]):
    @overload
    def __get__(
        self, instance: None, owner: Any
    ) -> InstrumentedAttribute[_T_co]:
        ...

    @overload
    def __get__(self, instance: object, owner: Any) -> _T_co:
        ...

    def __get__(
        self, instance: object | None, owner: Any
    ) -> InstrumentedAttribute[_T_co] | _T_co:
        ...


from typing import Protocol


class ParentProtocol(Protocol):
    name: Mapped[str]


class Parent:
    name: Mapped[str]


class Item:
    id: Mapped[int]
    parent: Mapped[Parent]


def check_parent_protocol(p: ParentProtocol) -> str:
    return p.name


def check_parent(p: Parent) -> str:
    return p.name


check_parent(Parent)
check_parent(Parent())  # ok
check_parent_protocol(Parent)
check_parent_protocol(Parent())  # ok


class PropProtocol(Protocol):
    @property
    def parent(self) -> Mapped[ParentProtocol]:
        ...


def get_parent_name(child: PropProtocol) -> str:
    reveal_type(child.parent)
    return child.parent.name


get_parent_name(Item)
get_parent_name(Item())


class AttrProtocol(Protocol):
    parent: Mapped[ParentProtocol]


def get_parent_name_again(child: AttrProtocol) -> str:
    return child.parent.name


get_parent_name_again(Item)
get_parent_name_again(Item())

erictraut · 2023-09-08T21:22:19Z

erictraut
Sep 8, 2023
Maintainer

Question 1
This question involves the difference between nominal and structural types. A nominal type uses class hierarchies to determine type compatibility. The expression Parent has the type type[Parent] and is not a subtype of Parent, so this is not type compatible. Structural types (protocols) work differently. They define an interface, and a type is deemed compatible with the structural type if it complies with that interface. The ParentProtocol class defines an interface that includes a single attribute named name with a type of Mapped[str]. Values that comply with that interface are compatible. Instances of Parent surely satisfy this interface, but so does the class Parent itself. That's because it has a class variable named name with a type that is compatible with Mapped[str].

Mypy and pyright differ slightly in how they interpret class variables vs instance variables. Mypy, in particular, is a bit sloppy and inconsistent in how it handles these. There are multiple open issues in the mypy issue tracker on this topic. You can read more about this here. I suspect that the reason mypy rejects check_parent_protocol(Parent) is because it treats ParentProtocol.name as a pure instance variable as opposed to a regular class variable.

Question 2
When a descriptor (or a property, which is implemented as a descriptor) returns another descriptor, the runtime doesn't invoke the descriptor protocol a second time. Doing so could lead to infinite recursion. The runtime therefore invokes a descriptor only once. If that descriptor returns another descriptor, it doesn't invoke the second descriptor's __get__ method. Mypy appears to not model this correctly. Here's a simplified example that demonstrates this.

from typing import Any, reveal_type

class IntDescriptor:
    def __get__(self, instance: object | None, owner: Any) -> int:
        return 0

class Foo:
    @property
    def parent(self) -> IntDescriptor:
        return IntDescriptor()

f = Foo()
reveal_type(f.parent)  # pyright reveals "IntDescriptor", mypy reveals "int"
print(f.parent) # Runtime prints "IntDescriptor, which is consistent with pyright

Question 3
Mypy and pyright agree here. These are both type errors.

Pyright's nested error message is a bit misleading here. While Mapped[Parent] is a subtype of Mapped[ParentProtocol], the parent field is invariant because it is mutable. In light of this invariance, Mapped[Parent] and Mapped[ParentProtocol] are not compatible types.

I'll look at improving the error message to eliminate the confusion.

14 replies

CaselIT Sep 9, 2023
Author

Can Mapped by made Final automatically, without need to add Final[Mapped[...]]?

I'm not sure what you mean. Final doesn't apply to the type of the attribute (which in this case is Mapped[...]). The Final annotation is meta-information about the attribute independent of its type. So no, Final can't be applied automatically. You need to specify Final, just like you would need to provide a readonly modifier if the language supported such a keyword.

Ok, I was basically asking if something like this can be done (currently Final is just ignored)

_T = TypeVar("_T", covariant=True)
MappedFinal = Final[Mapped[_T]]

The current behavior is probably the most sensible one

Mypy does not seem to like final on protocols

Yeah, that's a mypy issue. You'll need to take it up with the mypy maintainers. I can't think of any justification for this limitation, but maybe they are thinking of some reason that's eluding me.

I noted that pyright currently requires you to assign a value to a Final variable within a protocol definition. That seems unnecessary, so I just changed the behavior to eliminate this check within protocols. That means you can remove the unnecessary = Mapping() within the two protocol class definitions in my above example.

makes sense, thanks. I'll try experimenting a bit more on this and look if the approach of using final can be documented within sqlalchemy. Thanks for the suggesting a solution

Regarding the "hybrid approach" mentioned above, would a new discussion or issue be a better place to further discuss it, or you think it would not be worth doing?

erictraut Sep 9, 2023
Maintainer

What you're calling a "hybrid approach" would result in too many false positive errors, so I think it's unlikely that pyright or mypy would implement it, at least in the manner you've suggested. As I mentioned, it's common in Python to assign a value to a class or instance variable later, outside of a class' definition. There is nothing wrong with this from a type perspective.

A while ago, I added a check to pyright called reportUninitializedInstanceVariable. It enforces that all instance variables declared in a class body are assigned a value either within the class body or the constructor. Perhaps that's the check you're looking for. To my knowledge, very few pyright users enable this feature because it forces a coding style that is not typical for most Python code bases. But if you're willing to adopt this coding pattern throughout your code, you will avoid the class of bug that you're concerned about here.

# pyright: reportUninitializedInstanceVariable=true

class W1:
    x: int # Error: x is not initialized

class W2:
    x: int = 2 # OK

class W3:
    def __init__(self):
        self.x = 3 # OK

CaselIT Sep 9, 2023
Author

Fair enough. I'm not sure I agree with "it's common in Python to assign a value to a class outside of a class' definition" but I guess the preference is to prefer false nagatives compared to false positive

I don't think that flag is useful to me, since it does not allow dataclasses (unless all variables have a default). Thanks anyway

erictraut Sep 9, 2023
Maintainer

I don't think that flag is useful to me, since it does not allow dataclasses

It does support dataclasses. The synthesized dataclass __init__ method assigns values to the instance variables that are missing a default, so these are considered initialized.

CaselIT Sep 9, 2023
Author

ok, understood

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pyright behavior with generic descriptors and protocols #5915

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Replies: 1 comment 14 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Pyright behavior with generic descriptors and protocols #5915

Uh oh!

Uh oh!

CaselIT Sep 8, 2023

Replies: 1 comment · 14 replies

Uh oh!

erictraut Sep 8, 2023 Maintainer

Uh oh!

CaselIT Sep 9, 2023 Author

Uh oh!

erictraut Sep 9, 2023 Maintainer

Uh oh!

Uh oh!

CaselIT Sep 9, 2023 Author

Uh oh!

erictraut Sep 9, 2023 Maintainer

Uh oh!

CaselIT Sep 9, 2023 Author

CaselIT
Sep 8, 2023

Replies: 1 comment 14 replies

erictraut
Sep 8, 2023
Maintainer

CaselIT Sep 9, 2023
Author

erictraut Sep 9, 2023
Maintainer

CaselIT Sep 9, 2023
Author

erictraut Sep 9, 2023
Maintainer

CaselIT Sep 9, 2023
Author