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

make mypy linting be more consistent with core and fix lints #591

Merged
merged 7 commits into from
Nov 10, 2022

Conversation

wanchaol
Copy link
Contributor

@wanchaol wanchaol commented Nov 2, 2022

Copy link
Contributor

@kwen2501 kwen2501 left a comment

Choose a reason for hiding this comment

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

Thanks a thousand for aligning Tau with PyTorch!
I submitted a PR #594 to fix the final lint other than those silenced ones.

@kwen2501
Copy link
Contributor

kwen2501 commented Nov 4, 2022

@wanchaol can I land this one? Any concern?

@wanchaol
Copy link
Contributor Author

wanchaol commented Nov 4, 2022

@wanchaol can I land this one? Any concern?

There're a bunch of ci failures, I need to resolve them before landing, do you need this soon? If so, I'll try to fix them soon and land it

@kwen2501
Copy link
Contributor

kwen2501 commented Nov 4, 2022

I see, no hurry, please take your time.
Just FYI -- the mypy errors are silenced, we can have more time to fix those.

@wanchaol wanchaol mentioned this pull request Nov 7, 2022
16 tasks
check.sh Outdated
@@ -65,7 +65,7 @@ flake8 pippy spmd test/spmd

# mypy spmd test/spmd
echo; echo "Running mypy ..."
mypy spmd pippy test examples
mypy spmd pippy examples
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kwen2501 after restricting more mypy linting with pytorch, there emerges tons of failures in tests, imo we shouldn't require strong type annotation in tests as that will slow down the dev efficiency, let me know if this not look good to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might also stay true for examples? I am changing mypy to only run under pippy and spmd

@wanchaol wanchaol merged commit d2422a4 into gh/wanchaol/4/base Nov 10, 2022
wanchaol added a commit that referenced this pull request Nov 10, 2022
ghstack-source-id: 35301cff60c9a260f72e008f29a49fc8a785f2e4
Pull Request resolved: #591
@wanchaol wanchaol deleted the gh/wanchaol/4/head branch November 10, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants