fix file preambles for Python scripts #357
Conversation
| # | ||
| # This code is distributed under the terms and conditions | ||
| # from the MIT License (MIT). | ||
| # |
There was a problem hiding this comment.
Blank line instead of blank comment, to separate the blocks.
There was a problem hiding this comment.
I think it's better to keep the entire preamble as a single comment.
If you use blank lines to separate comment blanks, identifying preambles in files (for scripting, etc.) becomes more difficult.
There was a problem hiding this comment.
It does not separate comment blocks – it separates preamble from actual code (imports).
| @@ -1,3 +1,13 @@ | |||
| # | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
The leading blank comment on top looks a bit weird, but I looked it up and coding can indeed be on the first or second line, so I guess this is fine: PEP263
I'd prefer to include the Python shebang always (any harm?).
There was a problem hiding this comment.
We can get rid of the leading blank line. It's not necessary.
Personally, I avoid the shebang unless the script is executable, simply because it is unnecessary otherwise.
I'd prefer to include the Python shebang always (any harm?).
Well, there's no harm (other than it being an eyesore). But consider this analogy: there's no harm in making all of our modules executable (chmod u+x), but we don't do it for the same reason (lack of necessity).
|
@piskvorky For the actual logic, see the release/check_preamble.py file. It's trivial. I think we can use it on gensim as-is. @menshikh-iv WDYT? |
|
@mpenkov why not? Though I still don't understand, why |
|
@piskvorky Is there a reason to even include a preamble in each file? We already have a separate license file. |
|
People copy around files, without necessarily copying entire projects. Not a big deal, just best practice. |
This follows on from comments on piskvorky/gensim#2610 (comment)
Main points:
git blamewill list everyone who has ever touched the file)I mostly copied the header from
smart_open/__init__.py. Let me know if you want me to change anything (I've got a script that takes care of it, so it's not a problem).Once you're happy, I can let this script loose on the gensim repo.