Skip to content

Maybe suppress all/most errors in unannotated functions #1370

Closed

Description

@gvanrossum

The semantic analysis can generate quite a few errors when run on a large unannotated code base.

When such errors refer to the body of an unannotated function, this violates people's expectations based on a simplified definition of gradual typing ("mypy only checks the body of a function when it's annotated").

This noise makes it less feasible to approach such a code base piecemeal (annotating one function/class/file at a time, then hunting down the errors).

OTOH there are perhaps some errors from semantic analysis that are useful.

We should do some kind of survey and either silence some of the noisiest errors from semantic analysis or (almost) all of them, depending on the outcome.

Activity

  1. added this to the milestone on Apr 12, 2016
  2. gvanrossum commented on Apr 12, 2016

    @gvanrossum
    MemberAuthor

    @ahaven you expressed interest in this idea earlier -- any messages that you find particularly noisy/annoting?

  3. drewhaven commented on Apr 13, 2016

    @drewhaven

    Here's the big ones:

    1. We have a number of functions that copy dicts and mutate a couple fields. For example,
    new_entry = dict(entry, a=1, b=2)
    

    The error it gives is because this constructor isn't stubbed (see #984), but it gives the error even inside of unannotated functions.

    1. Some of our test functions redefine helper functions multiple times within them.
    def test_some_reconstruct_cases(self):
        def reconstruct(...):
            ...
        ...
        def reconstruct(...):
            ...
        ...
    

    which gives "error: Name 'reconstruct' already defined". I'd like to go through and clean these up at some point, but I don't like that mypy's erroring on them right now because they're unannotated.

  4. JukkaL commented on Apr 13, 2016

    @JukkaL
    Collaborator

    The first error is from type checking. Mypy actually checks that functions and type objects are called with compatible argument count and keyword argument names, and if a stub is incorrect it can complain within dynamically typed functions. This could be easily fixed by giving all functions the type Any (for no checking), but these kinds of checks might actually be useful if our stubs were good enough. We could special case dict if it's the main source of these errors, though it would certainly be a hack.

    The latter is probably because some conditional definitions aren't supported within functions :-/ It shouldn't be very hard to get this right, just cumbersome.

  5. drewhaven commented on Apr 13, 2016

    @drewhaven

    For 1) I'd be happier with a correct stub for dict than working around the issue for unannotated functions :)

  6. gvanrossum commented on Apr 13, 2016

    @gvanrossum
    MemberAuthor
  7. JukkaL commented on Apr 14, 2016

    @JukkaL
    Collaborator

    We can't define a correct stub for dict, since PEP 484 isn't quite expressive enough. We could do one of these:

    1. Special case dict in the type checker.
    2. Implement a general plugin system for non-standard function signatures and use that for dict. The plugin would still be implemented as part of mypy, but would be less ad hoc.
    3. Somehow extend the type system to support specifying the signature of dict.

    The problem with annotating dict is that if there is a keyword argument, the key type must be a supertype of str.

    We could decide that an overload variant with **kwargs is not taken if another one matches and it doesn't have **kwargs. Then we could also support using non-default return types for __init__ (maybe only in stubs). I think that these are enough to define a good signature for dict:

    class dict(...):
        @overload
        def __init__(self) -> Dict[KT, VT]: ...
        @overload
        def __init__(self, x: Mapping[KT, VT]) -> Dict[KT, VT]: ...
        @overload
        def __init__(self, iter: Iterable[Tuple[KT, VT]]) -> Dict[KT, VT]: ...
        @overload
        def __init__(self, **kwargs: VT) -> Dict[str, VT]: ...
        @overload
        def __init__(self, x: Mapping[str, VT], **kwargs: VT) -> Dict[str, VT]: ...
        @overload
        def __init__(self, iter: Iterable[Tuple[str, VT]], **kwargs: VT) -> Dict[str, VT]: ...
    

    We could use union types for a more general signature for the **kwargs variants but it's probably not worth the added complexity.

    Alternatively, we could use the same **kwargs overloading rule and express the key type using a declarative constraint:

    class dict(...):
        # only two variants included in this example
        @overload
        def __init__(self) -> None: ...
        @overload
        def __init__(self, **kwargs: VT) -> None:
            # constraint: KT == str  # or use a decorator for constraints
            ...
    

    The constraint based approach could be generalized for other kinds of special signatures, but it would likely be harder to explain to users. A plugin system could use declarative constraints internally, though.

  8. gnprice commented on Apr 14, 2016

    @gnprice
    Collaborator

    My inclination would be to prioritize people's ability to adopt
    type-checking on a large existing file piecemeal. We should fix the dict
    stub one way or another (this is probably something worth making its own
    ticket for), but there will always be more things that either are wrong in
    a stub somewhere (especially in the large expanse of third-party
    libraries), or are confusing code patterns that confuse semantic analysis
    and mypy rightly gives an error for. So it's best if those don't create a
    barrier to annotating one unrelated function in the same file.

    I think there is a use case for applying the checks like compatible
    argument count and keyword argument names in unannotated functions, but I
    think it'd be best to leave that opt-in. In fact I think --implicit-any
    probably already serves this use case. If it doesn't in some way, perhaps
    there's space for another intermediate level of checking specified by
    another flag.

    On Thu, Apr 14, 2016 at 5:09 AM, Jukka Lehtosalo [email protected]
    wrote:

    We can't define a correct stub for dict, since PEP 484 isn't quite
    expressive enough. We could do one of these:

    1. Special case dict in the type checker.
    2. Implement a general plugin system for non-standard function
      signatures and use that for dict. The plugin would still be
      implemented as part of mypy, but would be less ad hoc.
    3. Somehow extend the type system to support specifying the signature
      of dict.

    The problem with annotating dict is that if there is a keyword argument,
    the key type must be a supertype of str.

    We could decide that an overload variant with *_kwargs is not taken if
    another one matches and it doesn't have *_kwargs. Then we could also
    support using non-default return types for init (maybe only in
    stubs). I think that these are enough to define a good signature for dict:

    class dict(...):
    @overload
    def init(self) -> Dict[KT, VT]: ...
    @overload
    def init(self, x: Mapping[KT, VT]) -> Dict[KT, VT]: ...
    @overload
    def init(self, iter: Iterable[Tuple[KT, VT]]) -> Dict[KT, VT]: ...
    @overload
    def init(self, *_kwargs: VT) -> Dict[str, VT]: ...
    @overload
    def init(self, x: Mapping[str, VT], *_kwargs: VT) -> Dict[str, VT]: ...
    @overload
    def init(self, iter: Iterable[Tuple[str, VT]], **kwargs: VT) -> Dict[str, VT]: ...

    We could use union types for a more general signature for the **kwargs
    variants but it's probably not worth the added complexity.

    Alternatively, we could use the same **kwargs overloading rule and
    express the key type using a declarative constraint:

    class dict(...):
    # only two variants included in this example
    @overload
    def init(self) -> None: ...
    @overload
    def init(self, **kwargs: VT) -> None:
    # constraint: KT == str # or use a decorator for constraints
    ...

    The constraint based approach could be generalized for other kinds of
    special signatures, but it would likely be harder to explain to users. A
    plugin system could use declarative constraints internally, though.

    —
    You are receiving this because you are subscribed to this thread.
    Reply to this email directly or view it on GitHub
    #1370 (comment)

  9. JukkaL commented on Apr 14, 2016

    @JukkaL
    Collaborator

    I'm leaning towards agreeing with @gnprice. --implicit-any is likely a better option, at least once it's file-level, and we can revisit this later and enable additional checks once false positive rate is reasonable. Disabling all errors (that we can easily filter out) in unannotated functions seems like the best option for now, and it shouldn't be a lot of work to implement.

  10. gvanrossum commented on Apr 14, 2016

    @gvanrossum
    MemberAuthor

    Just to be clear, the action items appear to be (a) fix the mixed dict signature, (b) suppress errors from semantic analysis in unannotated functions. Right?

  11. JukkaL commented on Apr 15, 2016

    @JukkaL
    Collaborator

    Yeah, and if (a) isn't good enough (e.g., if there are many functions that generate false positives), we can also suppress errors from type checking in unannotated functions.

  12. gvanrossum commented on May 20, 2016

    @gvanrossum
    MemberAuthor

    We've started suppressing all errors from semantic analysis in unannotated functions. Given the title of this issue I am closing it. (Though it seems an odd duplicate if #1415.)

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions