Skip to content

mypy has implemented pep-681 incorrectly and does not intepret descriptors correctly #14868

Closed
@zzzeek

Description

@zzzeek

Hi -

Now that mypy has pep-681 support, they are making a mistake that we spent a lot of time with the pyright people working out.

This SQLAlchemy mapping and usage is valid:

from __future__ import annotations

from sqlalchemy.orm import DeclarativeBase
from sqlalchemy.orm import Mapped
from sqlalchemy.orm import mapped_column
from sqlalchemy.orm import MappedAsDataclass


class Base(MappedAsDataclass, DeclarativeBase):
    pass


class A(Base):
    __tablename__ = "a"

    id: Mapped[int] = mapped_column(primary_key=True, init=False)
    data: Mapped[str]


a1 = A(data='some data')

however Mypy reports:

test3.py:20: error: Argument "data" to "A" has incompatible type "str"; expected "Mapped[str]"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

this is not correct. Mapped[] is a descriptor, so the appropriate value that should be set is a string, not a descriptor object.

Here's a complete example using only typing as an import

from __future__ import annotations

from typing import Any
from typing import Generic
from typing import Optional
from typing import overload
from typing import TYPE_CHECKING
from typing import TypeVar
from typing import Union

from typing_extensions import dataclass_transform

_T = TypeVar("_T")


def model_field(
    *,
    default: Optional[Any] = None,
    init: bool = True,
) -> Any:
    raise NotImplementedError()


@dataclass_transform(
    eq_default=True, order_default=True, field_specifiers=(model_field,)
)
class ModelBase:
    def __init_subclass__(
        cls,
        *,
        init: bool = True,
        frozen: bool = False,
        eq: bool = True,
        order: bool = True,
    ):
        ...


class Mapped(Generic[_T]):
    if TYPE_CHECKING:

        @overload
        def __get__(self, instance: None, owner: Any) -> Mapped[_T]:
            ...

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

        def __get__(
            self, instance: Optional[object], owner: Any
        ) -> Union[Mapped[_T], _T]:
            ...

        def __set__(self, instance: Any, value: _T) -> None:
            ...

        def __delete__(self, instance: Any) -> None:
            ...


class Customer(ModelBase):
    a: int


c1 = Customer(a=5)


class AlsoCustomer(ModelBase):
    a: Mapped[int]


# this is correct under pyright
c2 = AlsoCustomer(a=5)


# this is an error under pyright
c3 = AlsoCustomer(a="some string")

We had very involved discussions about this here: microsoft/pyright#2958 where the authors of pep-681 decided only to add a note about this here: https://peps.python.org/pep-0681/#descriptor-typed-field-support

While I was disappointed they would not add this language to the spec explicitly, they state it in that paragraph:

When enabled, the type of each parameter on the synthesized init method corresponding to a descriptor-typed field would be the type of the value parameter to the descriptor’s set method rather than the descriptor type itself. Similarly, when setting the field, the set value type would be expected. And when getting the value of the field, its type would be expected to match the return type of get.

This idea was based on the belief that dataclass did not properly support descriptor-typed fields. In fact it does, but type checkers (at least mypy and pyright) did not reflect the runtime behavior which led to our misunderstanding. For more details, see the Pyright bug.

That is, Python dataclasses when a descriptor is used as the left hand type, the type of the actual attribute passed to init must be the contained datatype, not the descriptor type.

We at SQLAlchemy spent weeks back and forth with the pep-681 authors on this and I am a little bit terrified that because this language wasn't included, Mypy did it exactly the wrong way.

I am very hopeful that the Mypy authors can correct this behavior else we will have to get our users to disable pep-681 support for mypy.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions