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

Give self argument the correct type when using check_untyped_defs #7530

Merged
merged 4 commits into from
Sep 19, 2019

Conversation

msullivan
Copy link
Collaborator

This is accomplished by updating the helper that constructs a callable type
from a func def to fill in the self argument.

This will make check_untyped_defs a lot more useful.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks very good. Could you please add Fixes #... to the PR description for relevant issues? I believe these are:
#7309
#5401
#4637
#1514

@classmethod
def bar(cls):
cls.baz() # E: "Type[Foo]" has no attribute "baz"
[builtins fixtures/classmethod.pyi]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add also tests for two other reported problems:

  • An attribute initialized with 0 in untyped __init__ has revealed type int in another untyped method
  • Descriptors also work as expected in untyped methods.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if an attribute is initialized to None in __init__ the inferred type should probably be either Optional[Any] or Any (the first one may generate many false positives). If an attribute is initialized to [], the inferred type should probably be List[Any], and I think that we shouldn't require a type annotation.

@The-Compiler
Copy link
Contributor

As a data point: In my project I just introduced check_untyped_defs a week ago - after the upgrade to 0.740 I now see 469 new errors 😆

Thanks for this, though! I have some work ahead of me now, but it indeed seems to make mypy a lot more useful for codebases which aren't fully annotated yet.

facebook-github-bot pushed a commit to pytorch/pytorch that referenced this pull request Oct 27, 2019
…27445)

Summary:
`__iter__` methods are supposed to return iterators (https://docs.python.org/3/reference/datamodel.html#object.__iter__), but some of them are typed to return iterables, which is too general. This results in error messages such as `Iterable[Module[Any]]" has no attribute "__next__"` from Mypy. Technically this should also have caused a type error [here](https://github.com/pytorch/pytorch/blob/8f7020bbdbb5537bf1954cd252523cb17ab879b1/torch/nn/modules/container.py#L115), but due to a bug in Mypy type checking isn't working correctly in untyped methods (this will be fixed in the next release though: python/mypy#7530).
Pull Request resolved: #27445

Reviewed By: lerks

Differential Revision: D18113966

Pulled By: fmassa

fbshipit-source-id: c6261ac866f86df4328e6d2fdfca0625aa2d2492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants