Skip to content
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.

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

additional issue in recently fixed pep-681 fix discovered #15020

Closed
zzzeek opened this issue Apr 7, 2023 · 3 comments · Fixed by #15628
Closed

additional issue in recently fixed pep-681 fix discovered #15020

zzzeek opened this issue Apr 7, 2023 · 3 comments · Fixed by #15628
Labels
bug mypy got something wrong topic-dataclass-transform PEP 681

Comments

@zzzeek
Copy link

zzzeek commented Apr 7, 2023

hi, this is continuing from #14868, a user found another variant which points to something that likely needs to be adjusted in the fix just made in #15006.

same test case as #14868, but we add an additional alternative input type for the setter; the new logic does not interpret "optional" correctly (pyright works):

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 SpecialWrapper(Generic[_T]):
    """some special kind of type that is generic around a _T"""

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: Union[SpecialWrapper[_T], _T]) -> None:
            """the set method can accept values that are wrapped in the
            SpecialWrapper as well as the value directly.  this is what
            causes the mypy issue.

            """
            ...

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


class Customer(ModelBase):
    a: Mapped[Optional[str]] = model_field(default=None)

# works
c1 = Customer(a='asdf')

# fails
c2 = Customer(a=None)

where the second case, Optional is not unwrapping the inner type correctly:

$ mypy test4.py 
test4.py:77: error: Argument "a" to "Customer" has incompatible type "None"; expected "Union[SpecialWrapper[Optional[str]], str]"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

@zzzeek zzzeek added the bug mypy got something wrong label Apr 7, 2023
@zzzeek
Copy link
Author

zzzeek commented Apr 7, 2023

for reference this is the SQLAlchemy case

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



class User(MappedAsDataclass, DeclarativeBase):
    __tablename__ = "user_account"
    id: Mapped[int] = mapped_column(init=False)
    name: Mapped[str]
    fullname: Mapped[str | None] = mapped_column(default=None)


user = User(name="Me", fullname="Myself")  # good
user = User(name="Me")  # also good
user = User(
    name="Me", fullname=None
)

@Benjamin-T
Copy link

Any work being done on this?

@cdce8p
Copy link
Collaborator

cdce8p commented Jul 9, 2023

Opened #15628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-dataclass-transform PEP 681
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants