Skip to content

Remove shebang from non-executable files in torch.distributed #27285

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
pietern wants to merge 2 commits into from
Closed

Remove shebang from non-executable files in torch.distributed #27285

pietern wants to merge 2 commits into from

Conversation

@pietern
Copy link
Contributor

@pietern pietern commented Oct 3, 2019 •
edited
Loading

Stack from ghstack:

Differential Revision: D17808207

@pytorchbot pytorchbot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Oct 3, 2019
This was referenced Oct 3, 2019
Closed
Closed
Closed
Closed
Closed
Closed
xuhdev
xuhdev reviewed Oct 3, 2019
View reviewed changes
Copy link
Collaborator

@xuhdev xuhdev left a comment

Choose a reason for hiding this comment

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

These shebangs might be used to indicate they must run on Python 3, not Python 2

@zhaojuanmao
Copy link
Contributor

zhaojuanmao commented Oct 3, 2019

right, they are used to indicate codes run on python3 only

@mrshenli
Copy link
Contributor

mrshenli commented Oct 4, 2019

rpc_test.py also has this shebang, which is not a runnable either. @pietern

@xuhdev @zhaojuanmao

These shebangs might be used to indicate they must run on Python 3, not Python 2

IIUC, this shebang won't print warning or error messages even if it is executed using PY2. If the shebang is only used for documentation purpose, shall we just use normal comments instead? I checked how jit handled this. Instead of adding a shebang, jit_test_py3.py explicitly added that in the name.

@pietern
Copy link
Contributor Author

pietern commented Oct 4, 2019

I agree -- the hash bang is an intent to make a file executable.

@mrshenli Can you stamp this one?

@xuhdev
Copy link
Collaborator

xuhdev commented Oct 4, 2019

@mrshenli A normal comment is fine. I'm just pointing out that using shebang is one kind of convention. You can also enable lint checks against such style in .flake8 by removing EXE001.

This was referenced Oct 7, 2019
Closed
Closed
Closed
Closed
Closed
Closed
xuhdev added a commit to xuhdev/pytorch that referenced this pull request Oct 8, 2019
@xuhdev
Enable EXE001 flake8 check.
b1ee24b 
According to pytorch#27285 , seems we do not intend to use shebang as an indication of Python version, thus
we enable EXE001 flake8 check.
For violations, we either remove shebang from non-executable Python scripts or grant them executable permission.
@xuhdev xuhdev mentioned this pull request Oct 8, 2019
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 8, 2019

@pietern merged this pull request in f597926.

facebook-github-bot pushed a commit that referenced this pull request Oct 9, 2019
@xuhdev @facebook-github-bot
Enable EXE001 flake8 check. (#27560)
987e37b 
Summary:
According to #27285 , seems we do not intend to use shebang as an indication of Python version, thus
we enable EXE001 flake8 check.
For violations, we either remove shebang from non-executable Python scripts or grant them executable permission.
Pull Request resolved: #27560

Differential Revision: D17831782

Pulled By: ezyang

fbshipit-source-id: 6282fd3617b25676a6d959af0d318faf05c09b26
@facebook-github-bot facebook-github-bot deleted the gh/pietern/40/head branch October 28, 2019 22:18
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
@pietern
Remove shebang from non-executable files in torch.distributed
c0c5927 
Summary: Pull Request resolved: pytorch#27285

Test Plan: Imported from OSS

Differential Revision: D17808207

Pulled By: pietern

fbshipit-source-id: 6141c1783e3a6f448a298275120db1f254b42b2a
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
@xuhdev
Enable EXE001 flake8 check. (pytorch#27560)
cb676d3 
Summary:
According to pytorch#27285 , seems we do not intend to use shebang as an indication of Python version, thus
we enable EXE001 flake8 check.
For violations, we either remove shebang from non-executable Python scripts or grant them executable permission.
Pull Request resolved: pytorch#27560

Differential Revision: D17831782

Pulled By: ezyang

fbshipit-source-id: 6282fd3617b25676a6d959af0d318faf05c09b26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xuhdev xuhdev xuhdev left review comments

@mrshenli mrshenli mrshenli approved these changes

@apaszke apaszke Awaiting requested review from apaszke

Assignees

No one assigned

Labels

Merged oncall: distributed Add this issue/PR to distributed oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@pietern @zhaojuanmao @mrshenli @xuhdev @facebook-github-bot @pytorchbot @mruberry