# Update Python coding standard based on RFC-162

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
5.2
• Team:
SQuaRE

#### Description

RFC-162 proposes that the Python coding standard should be rewritten in terms of PEP-8. Kian-Tat Lim has requested that the proposed new text of the coding standard be submitted for his approval prior to incorporation into developer.lsst.io, presumably as a pull request. This ticket covers that rewriting.

#### Activity

Hide
Tim Jenness added a comment -

I like the new version (N251 vs E251 noted on PR) and I'm happy for it to be merged with one caveat. Re-reading it I'm now more convinced that we have the order in section 1 wrong. I really think it should be:

1. We use PEP8
2. These are the exceptions to PEP8
3. This is how you can check whether you are compliant
4. This is how you can automatically fix up your code.

Out of scope for this work, I feel that section 10 "Idiomatic Python" is not named properly. It covers two distinct topics. It starts by giving general programming advice, but it then it describes mandatory coding approaches. I think hiding the "You MUST do THIS" parts of the document in an "Idioms" section is underselling it.

Show
Tim Jenness added a comment - I like the new version (N251 vs E251 noted on PR) and I'm happy for it to be merged with one caveat. Re-reading it I'm now more convinced that we have the order in section 1 wrong. I really think it should be: We use PEP8 These are the exceptions to PEP8 This is how you can check whether you are compliant This is how you can automatically fix up your code. Out of scope for this work, I feel that section 10 "Idiomatic Python" is not named properly. It covers two distinct topics. It starts by giving general programming advice, but it then it describes mandatory coding approaches. I think hiding the "You MUST do THIS" parts of the document in an "Idioms" section is underselling it.
Hide
Jonathan Sick added a comment -

Much thanks for your patient review Tim Jenness. I switched the order of Section 1 around as you suggest and it does work better. Also fixed the E251 error.

As for the Idiomatic Python section; I’m happy to try to fix that before merging. I think part of the problem may be that some of the sections don’t have SHOULD/MUST -type language in their headers yet because they were guidelines creating from passing mentions in the original style guide. For example "Use ‘as' when catching an exception” could probably be “The ‘as’ syntax MUST be used when catching an exception." Opinions? Or should we just merge?

Show
Jonathan Sick added a comment - Much thanks for your patient review Tim Jenness . I switched the order of Section 1 around as you suggest and it does work better. Also fixed the E251 error. As for the Idiomatic Python section; I’m happy to try to fix that before merging. I think part of the problem may be that some of the sections don’t have SHOULD/MUST -type language in their headers yet because they were guidelines creating from passing mentions in the original style guide. For example "Use ‘as' when catching an exception” could probably be “The ‘as’ syntax MUST be used when catching an exception." Opinions? Or should we just merge?
Hide
Jonathan Sick added a comment - - edited

In addition, I’ll repeat here a comment from the PR on likely future tickets on this page:

1. PEP 8 naming (RFC-97)
2. super being more acceptable
3. Elimination of non-PEP 8 whitespace rules in https://developer.lsst.io/v/DM-5456/coding/python_style_guide.html#keyword-assignment-operators-should-be-surrounded-by-a-space-when-statements-appear-on-multiple-lines
4. Eliminate mention of test suites in non-test modules: https://developer.lsst.io/v/DM-5456/coding/python_style_guide.html#standard-code-order-should-be-followed
5. Comments, even for private APIs, should be docstrings: https://developer.lsst.io/v/DM-5456/coding/python_style_guide.html#docstrings-should-be-written-for-all-public-modules-functions-classes-and-methods
Show
Jonathan Sick added a comment - - edited In addition, I’ll repeat here a comment from the PR on likely future tickets on this page: PEP 8 naming ( RFC-97 ) super being more acceptable Elimination of non-PEP 8 whitespace rules in https://developer.lsst.io/v/DM-5456/coding/python_style_guide.html#keyword-assignment-operators-should-be-surrounded-by-a-space-when-statements-appear-on-multiple-lines Eliminate mention of test suites in non-test modules: https://developer.lsst.io/v/DM-5456/coding/python_style_guide.html#standard-code-order-should-be-followed Comments, even for private APIs, should be docstrings: https://developer.lsst.io/v/DM-5456/coding/python_style_guide.html#docstrings-should-be-written-for-all-public-modules-functions-classes-and-methods
Hide
John Parejko added a comment -

One more addition for a possible future ticket is to make this bit match PEP-257 (single line docstrings should be a single line, not end on a new line):

https://developer.lsst.io/coding/python_style_guide.html#docstrings-should-be-begin-with-and-terminate-with-on-its-own-line

Show
John Parejko added a comment - One more addition for a possible future ticket is to make this bit match PEP-257 (single line docstrings should be a single line, not end on a new line): https://developer.lsst.io/coding/python_style_guide.html#docstrings-should-be-begin-with-and-terminate-with-on-its-own-line
Hide
Tim Jenness added a comment -

Merge what you've got. I probably need to modify the idioms section anyhow when I work on python 3 support.

Show
Tim Jenness added a comment - Merge what you've got. I probably need to modify the idioms section anyhow when I work on python 3 support.

#### People

Assignee:
Jonathan Sick
Reporter:
Tim Jenness
Reviewers:
Tim Jenness
Watchers:
John Parejko, Jonathan Sick, Kian-Tat Lim, Tim Jenness