Add --check-untyped-defs to mypy.ini and test suite by rgommers · Pull Request #37594 · pytorch/pytorch

rgommers · 2020-04-30T18:13:11Z

Also move the ignores for imports to the bottom in mypy.ini, those are much less interesting - start with the stuff people want to work on.

Second commit tests the instructions: remove an ignore, fix the issue.

Also move the ignores for imports to the bottom in mypy.ini, those are much less interesting - start with the stuff people want to work on.

ntenenz · 2020-05-01T07:14:44Z

+2. Inline type annotations for all Python code, _except_ if there are too many
+   overloads for functions/methods - in that case a stub file should be
+   preferred (what's too many is a bit of a judgement call, I'd suggest two per
+   function is a reasonable threshold).


One of the benefits of type hints is that they are self-documenting. For this reason, I'd personally be a bit more aggressive in the number of permissible @overloads permitted before going to stubs. Unfortunately, I don't have a firm number in mind.

If feasible, what might be beneficial is an analysis of the distribution of @overloads per function/method to inform the decision.

Right now I don't see any that exceed the threshold. torch/nn/modules/container.pyi has some double overloads (__init__, __getitem__). I expect the number of overloads to grow significantly though when people want to make signatures more specific - things like Union[Iterable[Tensor], Iterable[dict]] and Union[int, List[int], Size] may turn into overloads.

That said, point taken, I'll bump it to 3.

#35566 is another case where we may not be able to put things in code, and have to move it to stubs.

ezyang · 2020-05-01T14:33:40Z

This LGTM. @rgommers I'll let you make the small updates first? Another place these docs could live is in the wiki, where you don't have to go through landing cycle as is the case in the main repo.

rgommers · 2020-05-01T14:38:51Z

I'll let you make the small updates first?

yep

Another place these docs could live is in the wiki, where you don't have to go through landing cycle as is the case in the main repo.

That makes sense to me. I only used this because mypy-README already existed. It looks a bit out of place at the root of the repo; happy to delete it completely in this PR and move the content to the wiki.

ezyang · 2020-05-01T14:55:47Z

That makes sense to me. I only used this because mypy-README already existed. It looks a bit out of place at the root of the repo; happy to delete it completely in this PR and move the content to the wiki.

SGTM!

dr-ci · 2020-05-06T15:37:27Z

💊 Build failures summary and remediations

As of commit 4c6fa06 (more details on the Dr. CI page):

1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following build failures do not appear to be due to upstream breakages:

pytorch_linux_xenial_py3_6_gcc5_4_ge_config_profiling_test (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

May 06 21:13:22 NameError: name 'enable_profiling_mode' is not defined

May 06 21:13:22   test_init_ops (jit.test_unsupported_ops.TestUnsupportedOps) ... ok (0.019s) 
May 06 21:13:22   test_ops_bound_in_functional (jit.test_unsupported_ops.TestUnsupportedOps) ... ok (0.011s) 
May 06 21:13:22   test_tensor_options_behavior_mismatch (jit.test_unsupported_ops.TestUnsupportedOps) ... ok (0.003s) 
May 06 21:13:22  
May 06 21:13:22 ====================================================================== 
May 06 21:13:22 ERROR [0.006s]: test_profiling_merge (test_jit.TestScript) 
May 06 21:13:22 ---------------------------------------------------------------------- 
May 06 21:13:22 Traceback (most recent call last): 
May 06 21:13:22   File "/var/lib/jenkins/workspace/test/test_jit.py", line 3623, in test_profiling_merge 
May 06 21:13:22     with enable_profiling_mode(): 
May 06 21:13:22 NameError: name 'enable_profiling_mode' is not defined 
May 06 21:13:22  
May 06 21:13:22 ---------------------------------------------------------------------- 
May 06 21:13:22 Ran 2457 tests in 146.152s 
May 06 21:13:22  
May 06 21:13:22 FAILED (errors=1, skipped=68, expected failures=1) 
May 06 21:13:22  
May 06 21:13:22 Generating XML reports... 
May 06 21:13:22 Generated XML report: test-reports/python-unittest/TEST-jit.test_async.TestAsync-20200506211056.xml 
May 06 21:13:22 Generated XML report: test-reports/python-unittest/TEST-jit.test_autodiff_subgraph_slicing.TestAutodiffSubgraphSlicing-20200506211056.xml 
May 06 21:13:22 Generated XML report: test-reports/python-unittest/TEST-jit.test_builtins.TestBuiltins-20200506211056.xml

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 13 times.

facebook-github-bot

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

rgommers · 2020-05-06T21:36:22Z

While finishing the "task list" for completing the type annotations I found it was better to add --check-untyped-defs to mypy.ini and the test suite now, so I changed the scope of this PR. There were another ~500 errors that that uncovered, so I added a bunch more ignore_errors = True.

The guide of how to add type annotations and ~180 tasks for it are now in https://github.com/pytorch/pytorch/wiki/Guide-for-adding-type-annotations-to-PyTorch. Please have a look at that as well if you see any fundamental problem.

I think when this PR is merged we're good to start tackling those tasks.

facebook-github-bot · 2020-05-07T14:12:39Z

@ezyang merged this pull request in 46ed334.

Summary: Also move the ignores for imports to the bottom in `mypy.ini`, those are much less interesting - start with the stuff people want to work on. Second commit tests the instructions: remove an ignore, fix the issue. Pull Request resolved: pytorch#37594 Differential Revision: D21434858 Pulled By: ezyang fbshipit-source-id: 4f1a6868cdb4cb59d072bcf105f48c3a5ba3ff98

rgommers added 2 commits April 30, 2020 19:59

Document how to run and improve type annotations better.

1c5de39

Also move the ignores for imports to the bottom in mypy.ini, those are much less interesting - start with the stuff people want to work on.

Remove ignore_errors from mypy.ini for torch.serialization

6257423

rgommers added the module: typing Related to mypy type annotations label Apr 30, 2020

rgommers requested a review from ezyang April 30, 2020 18:13

pytorchbot added the open source label Apr 30, 2020

rgommers mentioned this pull request Apr 30, 2020

Make it possible to use mypy to typecheck code that uses PyTorch #16574

Closed

ntenenz reviewed May 1, 2020

View reviewed changes

ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 5, 2020

rgommers added 3 commits May 6, 2020 17:12

Update mypy-README

aa55180

Update mypy.ini and mypy test to add --check-untyped-defs

99131a4

Finish task list in mypy-README.md

e4142b3

rgommers requested a review from apaszke as a code owner May 6, 2020 15:24

rgommers changed the title ~~Document how to run and improve type annotations better~~ Add --check-untyped-defs to mypy.ini and test suite May 6, 2020

Revert change I didn't mean to commit (build workaround)

864aec1

rgommers added 3 commits May 6, 2020 20:28

Merge branch 'master' into mypy-test

5d61fbf

Minor fixes

87bb662

Add sparse_coo to torch/__init__.pyi, and fix one mistake

4c6fa06

ezyang approved these changes May 6, 2020

View reviewed changes

facebook-github-bot reviewed May 6, 2020

View reviewed changes

facebook-github-bot closed this in 46ed334 May 7, 2020

facebook-github-bot added the merged label May 7, 2020

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --check-untyped-defs to mypy.ini and test suite#37594

Add --check-untyped-defs to mypy.ini and test suite#37594
rgommers wants to merge 9 commits intopytorch:masterfrom
rgommers:mypy-test

rgommers commented Apr 30, 2020

Uh oh!

ntenenz May 1, 2020

Uh oh!

rgommers May 1, 2020

Uh oh!

ezyang May 1, 2020

Uh oh!

Uh oh!

ezyang commented May 1, 2020

Uh oh!

rgommers commented May 1, 2020

Uh oh!

ezyang commented May 1, 2020

Uh oh!

dr-ci Bot commented May 6, 2020 •

edited

Loading

Uh oh!

facebook-github-bot left a comment

Uh oh!

rgommers commented May 6, 2020

Uh oh!

facebook-github-bot commented May 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

rgommers commented Apr 30, 2020

Uh oh!

ntenenz May 1, 2020

Choose a reason for hiding this comment

Uh oh!

rgommers May 1, 2020

Choose a reason for hiding this comment

Uh oh!

ezyang May 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ezyang commented May 1, 2020

Uh oh!

rgommers commented May 1, 2020

Uh oh!

ezyang commented May 1, 2020

Uh oh!

dr-ci Bot commented May 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 Build failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_linux_xenial_py3_6_gcc5_4_ge_config_profiling_test (1/1)

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

rgommers commented May 6, 2020

Uh oh!

facebook-github-bot commented May 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dr-ci Bot commented May 6, 2020 •

edited

Loading