add type annotations to torch.nn.modules.module by guilhermeleobas · Pull Request #49045 · pytorch/pytorch

guilhermeleobas · 2020-12-08T22:07:31Z

Fixes #49044

dr-ci · 2020-12-08T22:08:56Z

💊 CI failures summary and remediations

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

💚 💚 Looks good so far! There are no failures yet. 💚 💚

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 to the (internal) Dr. CI Users group.

This comment has been revised 33 times.

codecov · 2020-12-09T01:37:08Z

Codecov Report

Merging #49045 (b9382f2) into master (4774c68) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #49045      +/-   ##
==========================================
- Coverage   80.72%   80.72%   -0.01%     
==========================================
  Files        1904     1904              
  Lines      206764   206764              
==========================================
- Hits       166908   166907       -1     
- Misses      39856    39857       +1

rgommers · 2020-12-12T22:22:49Z

@guilhermeleobas could you also add comments here about the ignores you added? That helps both with review (reproducing the failures and using reveal_type is my alternative), and when the next person has to touch that bit of code.

rgommers · 2020-12-12T22:28:05Z

Also, in your next commit message, can you add that it closes gh-48640?

guilhermeleobas · 2020-12-15T22:00:43Z

Hi @rgommers, thanks for the review. I've addressed your comments

rgommers

Looks good now, thanks @guilhermeleobas.

CI notifications seem stuck, but CI is done - see https://app.circleci.com/pipelines/github/pytorch/pytorch?branch=pull%2F49045. Failures are unrelated.

walterddr · 2020-12-30T00:12:11Z

why cant we just use different variable names?

Yes, that works too

walterddr · 2020-12-30T00:19:37Z

I am not python expert. but cant we use del load to remove the function reference itself? (after all the load has been done, yes?)

Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>

guilhermeleobas · 2020-12-31T17:30:31Z

Do not merge this PR until one checks if the annotations introduce any regression. See:
#49564 (comment)

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

walterddr

picky back on this PR - i dont think there's any additional item that needs to be test with JIT-ability for this change. am I missing any? @malfet

walterddr · 2021-01-08T16:23:19Z

@@ -279,7 +279,7 @@ def parameters(self, recurse: bool = True) -> Iterator[Parameter]:

    def named_parameters(  # type: ignore[return]


shouldn't we remove this type ignore as well since we fixed the return Parameter

torch/distributed/nn/api/remote_module.py:280: error: Missing return statement [return]

lol. Got it.

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

walterddr

looks good. I dont see any additional JIT test required.

walterddr · 2021-01-09T19:12:50Z

actually please rebase master --> seems like merge conflict is preventing fb internal CI to run

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-01-12T01:07:07Z

@walterddr merged this pull request in 5f8e1a1.

Summary: Fixes pytorch#49044 Pull Request resolved: pytorch#49045 Reviewed By: malfet Differential Revision: D25767092 Pulled By: walterddr fbshipit-source-id: a81ba96f3495943af7bb9ee3e5fc4c94c690c405

guilhermeleobas added the module: typing Related to mypy type annotations label Dec 8, 2020

guilhermeleobas self-assigned this Dec 8, 2020

facebook-github-bot added cla signed oncall: distributed Add this issue/PR to distributed oncall triage queue fx labels Dec 8, 2020

pytorchbot added the open source label Dec 8, 2020

guilhermeleobas requested a review from rgommers December 9, 2020 15:38

guilhermeleobas marked this pull request as ready for review December 9, 2020 15:38

guilhermeleobas requested review from mingzhe09088, mrshenli, pritamdamania87, rohan-varma and zhaojuanmao as code owners December 9, 2020 15:38

guilhermeleobas removed request for mingzhe09088, mrshenli, pritamdamania87, rohan-varma and zhaojuanmao December 9, 2020 17:03

H-Huang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Dec 10, 2020

rgommers mentioned this pull request Dec 12, 2020

torch.nn.Module.named_parameters has wrong type annotation #48640

Closed

rgommers approved these changes Dec 19, 2020

View reviewed changes

rgommers requested review from ezyang and malfet December 19, 2020 19:16

walterddr reviewed Dec 30, 2020

View reviewed changes

add type annotations to torch.nn.modules.module

31a2abb

guilhermeleobas and others added 3 commits December 30, 2020 02:14

add comments explaining "type: ignore[...]"

901fd13

rename variables and replace inst

dc8f986

Add a few missing type annotations in nn/modules/module.py

a021139

Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>

guilhermeleobas mentioned this pull request Dec 31, 2020

add type annotations to torch.nn.modules.conv #49564

Closed

guilhermeleobas marked this pull request as draft December 31, 2020 17:30

facebook-github-bot reviewed Jan 4, 2021

View reviewed changes

guilhermeleobas added 2 commits January 8, 2021 15:52

Merge remote-tracking branch 'upstream/master' into module

8335398

Merge remote-tracking branch 'upstream/master' into module

818ba04

walterddr reviewed Jan 8, 2021

View reviewed changes

guilhermeleobas marked this pull request as ready for review January 8, 2021 16:50

facebook-github-bot reviewed Jan 8, 2021

View reviewed changes

walterddr approved these changes Jan 9, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into module

b9382f2

facebook-github-bot reviewed Jan 11, 2021

View reviewed changes

facebook-github-bot closed this in 5f8e1a1 Jan 12, 2021

facebook-github-bot added the Merged label Jan 12, 2021

guilhermeleobas deleted the module branch January 12, 2021 02:33

		@@ -279,7 +279,7 @@ def parameters(self, recurse: bool = True) -> Iterator[Parameter]:

		def named_parameters( # type: ignore[return]

Conversation

guilhermeleobas commented Dec 8, 2020

Uh oh!

dr-ci Bot commented Dec 8, 2020 • edited by facebook-github-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

codecov Bot commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rgommers commented Dec 12, 2020

Uh oh!

rgommers commented Dec 12, 2020

Uh oh!

guilhermeleobas commented Dec 15, 2020

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

walterddr Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas commented Dec 31, 2020

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr Jan 8, 2021

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Jan 8, 2021

Choose a reason for hiding this comment

Uh oh!

walterddr Jan 8, 2021

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr commented Jan 9, 2021

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dr-ci Bot commented Dec 8, 2020 •

edited by facebook-github-bot

Loading

codecov Bot commented Dec 9, 2020 •

edited

Loading