Skip to content

Enable typechecking of collect_env.py during CI #43062

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.

Sign up for GitHub

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

Jump to bottom
Closed
malfet wants to merge 1 commit into from
Closed

Enable typechecking of collect_env.py during CI #43062

malfet wants to merge 1 commit into from

Conversation

@malfet
Copy link
Contributor

@malfet malfet commented Aug 14, 2020

No type annotations can be added to the script, as it still have to be Python-2 compliant.
Make changes to avoid variable type redefinition.

@malfet malfet added module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Aug 14, 2020
@malfet malfet requested review from ezyang, rgommers and zou3519 August 14, 2020 16:22
@malfet
Enable typechecking of collect_env.py during CI
d67eb69 
No type annotations can be added to the script, as it still have to be Python-2 compliant
Make changes to avoid variable type redefinition
@malfet malfet force-pushed the malfet/enable-typechecking-for-collect_env branch from f766475 to d67eb69 Compare August 14, 2020 16:22
facebook-github-bot
facebook-github-bot reviewed Aug 14, 2020
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@zou3519
Copy link
Contributor

zou3519 commented Aug 14, 2020

No type annotations can be added to the script, as it still have to be Python-2 compliant.

Does adding python 2 style type annotations work? https://mypy.readthedocs.io/en/stable/python2.html
I'm not sure if running mypy (python 3) on Python 2 style annotations actually does anything or not.

@zou3519
Copy link
Contributor

zou3519 commented Aug 14, 2020

No type annotations can be added to the script, as it still have to be Python-2 compliant.

Does adding python 2 style type annotations work? https://mypy.readthedocs.io/en/stable/python2.html
I'm not sure if running mypy (python 3) on Python 2 style annotations actually does anything or not.

@dr-ci
Copy link

dr-ci bot commented Aug 14, 2020 •
edited
Loading

💊 CI failures summary and remediations

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

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

1 failure confirmed as flaky and can be ignored:

  • binary_windows_libtorch_3_7_cpu_release_test
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 or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 2 times.

@rgommers
Copy link
Collaborator

rgommers commented Aug 14, 2020

@malfet these changes are already in gh-42711, which is about to land.

@malfet
Copy link
Contributor Author

malfet commented Aug 14, 2020

@rgommers thank you for the ping, will chat with Ed in a few about it.

@malfet malfet deleted the malfet/enable-typechecking-for-collect_env branch August 14, 2020 19:58
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 14, 2020

@malfet merged this pull request in 64a7684.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@zou3519 zou3519 zou3519 approved these changes

@ezyang ezyang Awaiting requested review from ezyang

@rgommers rgommers Awaiting requested review from rgommers

Assignees

No one assigned

Labels

Merged module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@malfet @zou3519 @rgommers @facebook-github-bot @mruberry