Skip to content

Give self argument the correct type when using check_untyped_defs - #7530

Merged
msullivan merged 4 commits into
masterfrom
check-untyped-self
Sep 19, 2019
Merged

msullivan merged 4 commits into
masterfrom
check-untyped-self

Conversation

@msullivan

Copy link
Copy Markdown
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.

@ilevkivskyi ilevkivskyi left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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.

Comment thread mypy/typeops.py Outdated
Comment thread mypy/typeops.py
@The-Compiler

Copy link
Copy Markdown
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
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…ytorch#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/bd49d877c803dc2bc0fe6e11bef482f1805a3aeb/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: pytorch#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