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

mypy/typing false positives #576

Closed
ojii opened this issue Sep 20, 2019 · 16 comments
Closed

mypy/typing false positives #576

ojii opened this issue Sep 20, 2019 · 16 comments
Assignees
Labels
Typing Typing/stub/Mypy/PyRight related bugs.
Milestone

Comments

@ojii
Copy link

ojii commented Sep 20, 2019

Using attrs 6e7b9f2 and mypy 0.720:

from typing import List, Dict, Any, Union, Optional
import attr


@attr.s
class Foo:
    # OK
    str_list: List[str] = attr.ib(
        validator=attr.validators.deep_iterable(
            attr.validators.instance_of(str), attr.validators.instance_of(list)
        )
    )
    # Incompatible types in assignment (expression has type "Optional[List[_T]]", variable has type "Optional[List[str]]")
    maybe_str_list: Optional[List[str]] = attr.ib(
        validator=attr.validators.optional(
            attr.validators.deep_iterable(
                attr.validators.instance_of(str), attr.validators.instance_of(list)
            )
        )
    )
    # OK
    str_dict: Dict[str, Any] = attr.ib(
        validator=attr.validators.deep_mapping(
            attr.validators.instance_of(str),
            attr.validators.instance_of(object),
            attr.validators.instance_of(dict),
        )
    )
    # OK
    maybe_str_dict: Optional[Dict[str, Any]] = attr.ib(
        validator=attr.validators.optional(
            attr.validators.deep_mapping(
                attr.validators.instance_of(str),
                attr.validators.instance_of(object),
                attr.validators.instance_of(dict),
            )
        )
    )
    # Argument 1 to "instance_of" has incompatible type "Tuple[Type[str], Type[int]]"; expected "Union[Tuple[Type[<nothing>], ...], Type[<nothing>]]"
    str_int: Union[str, int] = attr.ib(
        validator=attr.validators.instance_of((str, int))
    )
    # (the error is shown twice for some reason)
    # Argument 1 to "instance_of" has incompatible type "Tuple[Type[str], Type[int]]"; expected "Union[Tuple[Type[<nothing>], ...], Type[<nothing>]]"
    # Argument 1 to "instance_of" has incompatible type "Tuple[Type[str], Type[int]]"; expected "Union[Tuple[Type[None], ...], Type[None]]"
    maybe_str_int: Optional[Union[str, int]] = attr.ib(
        validator=attr.validators.optional(attr.validators.instance_of((str, int)))
    )

deep_iterable seems to have an issue when combined with optional, instance_of doesn't like tuples of types.

@ojii ojii changed the title typing false positives mypy/typing false positives Sep 20, 2019
@hynek hynek added the Typing Typing/stub/Mypy/PyRight related bugs. label Sep 20, 2019
@hynek hynek added this to the 19.2 milestone Sep 20, 2019
@hynek
Copy link
Member

hynek commented Sep 20, 2019

Paging @mmaslowskicc @euresti @ethanhs 🤞

@euresti
Copy link
Contributor

euresti commented Sep 20, 2019

instance_of can be fixed with @overloads like this:

_T = TypeVar("_T")
_T2 = TypeVar("_T2")
_T3 = TypeVar("_T3")

@overload
def instance_of(type: Type[_T]) -> attr._ValidatorType[_T]:
    ...

@overload
def instance_of(type: Tuple[Type[_T]]) -> attr._ValidatorType[_T]:
    ...

@overload
def instance_of(type: Tuple[Type[_T], Type[_T2]]) -> attr._ValidatorType[Union[_T, _T2]]:
    ...

@overload
def instance_of(type: Tuple[Type[_T], Type[_T2], Type[_T3]]) -> attr._ValidatorType[Union[_T, _T2, _T3]]:
    ...

# Ok just fall back to Any if more than 3 values.
@overload
def instance_of(type: Tuple[type, ...]) -> attr._ValidatorType[Any]:
    ...

But how many do we add before giving up?

@hynek
Copy link
Member

hynek commented Sep 20, 2019

Three sounds like a good number?

@euresti
Copy link
Contributor

euresti commented Sep 20, 2019

I see the issue with deep_iterable but I don't know how to fix it.

deep_iterable is typed like this:

def deep_iterable(
    member_validator: _ValidatorType[_T],
    iterable_validator: Optional[_ValidatorType[_I]] = ...,
) -> _ValidatorType[_I]: ...

list is technically a Type[List[_T]]
which makes instance_of(list) a _ValidatorType[List[_T]]

Now when you add optional you end up with an Optional[List[_T]] which for some reason mypy won't coerce to an Optional[List[str]] even though it seems to do it just fine to do List[_T] to List[str]. I don't know why Optional is working against you there.

I see some paths forward:
A. Leave things as is. Users can always use typing.cast to work around the issue.
B. Have deep_iterable return a _ValidatorType[Any]. We lose some type checking but this is seriously weird code.
C. Spend several hours/days figuring out why the Optional[List[_T]] doesn't coerce to Optional[List[str]] issue in the mypy code and fix it.
D Spend several hours adding some code to the plugin to make deep_iterable return List[str] for this particular flow.

I don't have too much time so I cannot work on C or D.
A is free. B is just a simple stubs change.

@hynek
Copy link
Member

hynek commented Sep 22, 2019

Maybe A + report a bug to mypy?

@ojii
Copy link
Author

ojii commented Sep 24, 2019

A. Leave things as is. Users can always use typing.cast to work around the issue.

how can typing.cast be used in the case of a Optional[List[str]] in attrs to make mypy happy?

Answering my own question:

    maybe_str_list: Optional[List[str]] = attr.ib(
        validator=cast(
            attr._ValidatorType[Optional[List[str]]],
            attr.validators.optional(
                attr.validators.deep_iterable(
                    attr.validators.instance_of(str), attr.validators.instance_of(list)
                )
            ),
        )
    )```

quite the sight but it pacifies mypy

@euresti
Copy link
Contributor

euresti commented Sep 24, 2019

Yes, but you'll want to put attr._ValidatorType[Optional[List[str]]] in quotes because that's only defined in the stub and not at runtime.

Can the cast go on the attr.ib or does that break the plugin?

@euresti euresti self-assigned this Sep 25, 2019
euresti added a commit to euresti/attrs that referenced this issue Sep 25, 2019
hynek pushed a commit that referenced this issue Sep 26, 2019
* Typecheck stubs in CI and fix type errors.  Fixes #578

* Add overloads to instance_of definition.

Improves #576
@hynek
Copy link
Member

hynek commented Sep 26, 2019

Can you confirm instance_of is fixed, Jonas?

@hynek
Copy link
Member

hynek commented Sep 28, 2019

@euresti can I bother you to open a bug on mypy regarding the other issue please? 🐶 I believe we can close this bug once that's done.

@hawkowl
Copy link
Member

hawkowl commented Sep 30, 2019

ran into this, and attrs master fixes the instance_of being given a tuple.

@euresti
Copy link
Contributor

euresti commented Sep 30, 2019

Hmm. I now get no issues with the deep_iterable anymore using the attrs master and mypy 0.730

Maybe they fixed the Optional issue.

@hynek
Copy link
Member

hynek commented Sep 30, 2019

Can we close then?

@euresti
Copy link
Contributor

euresti commented Sep 30, 2019

Sure!

@euresti euresti closed this as completed Sep 30, 2019
@ojii
Copy link
Author

ojii commented Oct 1, 2019

is there a rough schedule for the next release of attrs? (I just want to know if I need to ship my workarounds or can wait a little bit for an attrs release, sorry if this sounds pushy)

@hynek
Copy link
Member

hynek commented Oct 1, 2019

As soon as #580 is merged. Dunno when someone finds time for review. ¯\_(ツ)_/¯

@ojii
Copy link
Author

ojii commented Nov 12, 2019

finally got around to update my library and can confirm these typing issues went away. thank you all for your hard work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing Typing/stub/Mypy/PyRight related bugs.
Projects
None yet
Development

No branches or pull requests

4 participants