Skip to content

[local linter] mypy_wrapper doesn't filter out distributed file #57967

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

Closed
driazati opened this issue May 10, 2021 · 8 comments
Closed

[local linter] mypy_wrapper doesn't filter out distributed file #57967

driazati opened this issue May 10, 2021 · 8 comments
Labels
module: flaky-tests Problem is a flaky test in CI triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@driazati
Copy link
Contributor

driazati commented May 10, 2021

mypy_wrapper's logic for excluding files doesn't totally match up with mypy itself (though the file below isn't listed in mypy.ini's excludes, so who knows what's really going on)

# this fails (but shouldn't lint this file at all)
python tools/mypy_wrapper.py $(pwd)/torch/nn/parallel/distributed.py

# but this works
mypy --config mypy.ini 
@facebook-github-bot facebook-github-bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label May 10, 2021
driazati added a commit to driazati/pytorch that referenced this issue May 10, 2021
@driazati driazati added module: flaky-tests Problem is a flaky test in CI triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module and removed oncall: distributed Add this issue/PR to distributed oncall triage queue labels May 10, 2021
@samestep samestep self-assigned this May 10, 2021
@samestep
Copy link
Contributor

Just tested this on commit c911c30 and it's still happening, so it was not fixed by #57998.

@samestep samestep reopened this May 12, 2021
@samestep
Copy link
Contributor

Ahh I understand now: I was thinking that this was about ignore_errors (which tools/mypy_wrapper.py does not need to handle), but it's actually about exclude, which we didn't have when I originally wrote our mypy wrapper, since it was added fairly recently in #55712.

@samestep
Copy link
Contributor

samestep commented May 12, 2021

I am a bit confused about what exactly is going on here, since torch/nn/parallel/distributed.py doesn't even match our current exclude regex:

exclude = torch/include/|torch/csrc/|torch/distributed/elastic/agent/server/api.py

(edit: whoops, I realize now that this fact is already noted in David's post above 😅)

krshrimali pushed a commit to krshrimali/pytorch that referenced this issue May 19, 2021
Summary:
Disabling until we fix pytorch#57967

Pull Request resolved: pytorch#57968

Pulled By: driazati

Reviewed By: samestep

Differential Revision: D28330226

fbshipit-source-id: 7ea130e0cf7b94959a7db18838d21e4711716625
@samestep samestep removed their assignment Aug 16, 2021
@seemethere
Copy link
Member

From triage meeting: solution should be a warning when mypy versions do not match

@samestep
Copy link
Contributor

@seemethere that sounds like a different issue from what this thread is about?

@seemethere
Copy link
Member

@seemethere that sounds like a different issue from what this thread is about?

We've concluded that the original issue stems from a mismatch of mypy versions, which is why a suggestion to warn when mypy versions do not match

@samestep
Copy link
Contributor

oh I see; but I thought we already did that?

plugins = mypy_plugins/check_mypy_version.py

plugins = mypy_plugins/check_mypy_version.py

@driazati driazati self-assigned this Aug 30, 2021
@driazati driazati removed their assignment Nov 14, 2021
@seemethere
Copy link
Member

This has already been completed, going to close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: flaky-tests Problem is a flaky test in CI triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

4 participants