Create a shared linting process for python file (including PEP 257)

[nj4710]

https://www.python.org/dev/peps/pep-0257/ pep257 can help us in improving the documentation of the code. It is also viable at this stage. What do you all think?

[miketaylr]

@nj4710 can you add some more context around what the work of this issue would be? For example, which modules or methods are missing docstrings, or which aren't following pep257 conventions?

Perhaps you could run the pep257 tool on the codebase and show the results here?

https://pypi.python.org/pypi/pep257

[nj4710]

This would be a long term issue. For starters, the scope of this issue would be to solve errors like these:

`pep257 webcompat/views.py`
`webcompat/views.py:1 at module level:
        D100: Missing docstring in public module
webcompat/views.py:40 in public function `shutdown_session`:
        D103: Missing docstring in public function
webcompat/views.py:45 in public function `before_request`:
        D103: Missing docstring in public function
webcompat/views.py:54 in public function `after_request`:
        D103: Missing docstring in public function
webcompat/views.py:62 in public function `token_getter`:
        D103: Missing docstring in public function
webcompat/views.py:69 in public function `format_date`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:76 in public function `login`:
        D103: Missing docstring in public function
webcompat/views.py:87 in public function `logout`:
        D103: Missing docstring in public function
webcompat/views.py:97 in public function `authorized`:
        D103: Missing docstring in public function
webcompat/views.py:117 in public function `file_issue`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:127 in public function `index`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:150 in public function `show_issues`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:159 in public function `create_issue`:
        D401: First line should be in imperative mood ('Create', not 'Creates')
webcompat/views.py:208 in public function `show_issue`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:219 in public function `me_redirect`:
        D401: First line should be in imperative mood ('Thi', not 'This')
webcompat/views.py:219 in public function `me_redirect`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:228 in public function `show_user_page`:
        D400: First line should end with a period (not ':')
webcompat/views.py:228 in public function `show_user_page`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:250 in public function `show_rate_limit`:
        D103: Missing docstring in public function
webcompat/views.py:268 in public function `download_file`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:278 in public function `get_test_helper`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:278 in public function `get_test_helper`:
        D200: One-line docstring should fit on one line with quotes (found 2)
webcompat/views.py:286 in public function `about`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:294 in public function `privacy`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:302 in public function `contributors`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:310 in public function `cssfixme`:
        D400: First line should end with a period (not 'l')
webcompat/views.py:310 in public function `cssfixme`:
        D300: Use """triple double quotes""" (found '''-quotes)
webcompat/views.py:316 in public function `log_csp_report`:
        D300: Use """triple double quotes""" (found '''-quotes)
`

We can have good first bugs to add missing docstrings for important classes or functions e.g in functions like shutdown_session, before_request docstring is missing. I ran the command for views.py. Can we run pep257 for the whole code base as well? @miketaylr

[karlcow]

@nj4710 Did you have specific troubles with the current code base? Were you missing anything or confused by some of the definition.

I'm asking because while it's good to fix the syntax and makes it clean and regular 💯 It's also often a big change for the code once. And also put in place recommendations in automatic linting tools in people's editor for not forgetting it the next time.

For example, I would be interested in knowing if we can find the right linting tool for our editors. I'm using SublimeText, Probably @magsout is using Atom? @zoepage I don't know what she is using.

• flake8 linting tool has a pep257 addon called pydocstyle.
• pep8 has just been renamed pycodestyle.
• autopep8
• anaconda
• and others.

I know that I'm often detecting things that are not seen by others in python, such as unused import, etc. because the tools are telling me it.

(which reminds me I should write a blog post on how I configure sublimetext for this work. And that would help me to clean up my config too. I'm not even sure which one I'm using really.)

[karlcow]

An example of what my sublimetext looks like when there is an error.

[nj4710]

No specific troubles(at least not yet :smile: ) but it took me more time to understand the meaning and relevance of a few functions. I understand that this will be a big change but we can take baby steps towards it. Maybe, we can add this to a milestone? Even I use sublime text and I am aware of one linting tool SublimeLinter Not sure about atom.

(Waiting to read your blog post, please post the link here as soon as you finish it :smile: )

[zoepage]

@karlcow

For example, I would be interested in knowing if we can find the right linting tool for our editors. I'm using SublimeText, Probably @magsout is using Atom? @zoepage I don't know what she is using.

Sublime Text 3 :)

[karlcow]

@nj4710 I think we can separate that in 2 issues.

• [ ] fix the actual docstrings when they are missing. #1482
• [ ] define a common setup in CONTRIBUTING.md for linting python files (including what should be done for pep257) (aka this issue 1473)

[karlcow]

I will change the topic for this issue.

[nj4710]

@karlcow That will be great!

[zoepage]

Are there any updates here? :)

[karlcow]

@zoepage Thanks for the reminder. Because I had a new computer and I helped @MDTsai to set up a couple of things. I will probably propose something after San Francisco work week.

I'm wondering about switching to flake8 instead of pep8 but I want to make sure that the cost of switching is worth for people.

I wonder how many of us are not using sublimetext for editing the code.

[karlcow]

@webcompat/collaborators What is your text editor for python code?

[zoepage]

Sublime3

[brizental]

Sublime 3

[denschub]

VIM!

[miketaylr]

sublime 3

[cch5ng]

sublime 3

[tsl143]

Sublime 3 <3

On Thu, Jun 29, 2017 at 5:21 AM, Carol Chung notifications@github.com wrote:

sublime 3

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/webcompat/webcompat.com/issues/1473#issuecomment-311823540, or mute the thread https://github.com/notifications/unsubscribe-auth/AFEoTJtSopcXYqrIwQea4AfPyhSQJskZks5sIucSgaJpZM4Mx44v .

-- Cheers, Trishul Goel

[karlcow]

So I guess I will come with a solution for sublime text 3. :) and Vim/emacs users will adjust. :)

[karlcow]

→ flake8 --statistics webcompat
webcompat/form.py:17:1: H306  imports not in alphabetical order (flask_wtf.flaskform, flask_wtf.file.fileallowed)
webcompat/form.py:27:1: H306  imports not in alphabetical order (webcompat.app, webcompat.api.uploads.upload)
webcompat/helpers.py:306:28: H403  multi line docstrings should end on a new line
webcompat/helpers.py:367:32: H403  multi line docstrings should end on a new line
webcompat/helpers.py:388:15: H403  multi line docstrings should end on a new line
webcompat/helpers.py:478:28: H403  multi line docstrings should end on a new line
webcompat/helpers.py:490:28: H403  multi line docstrings should end on a new line
webcompat/issues.py:13:1: H306  imports not in alphabetical order (webcompat.github, webcompat.form.build_formdata)
webcompat/views.py:22:1: H306  imports not in alphabetical order (form.proxy_report, form.get_form)
webcompat/views.py:33:1: H306  imports not in alphabetical order (webcompat.github, webcompat.db.user)
webcompat/views.py:34:1: H306  imports not in alphabetical order (webcompat.db.user, webcompat.db.session_db)
webcompat/api/endpoints.py:23:1: H306  imports not in alphabetical order (webcompat.limiter, webcompat.helpers.api_request)
webcompat/api/endpoints.py:250:1: H405  multi line docstring summary not separated with an empty line
webcompat/db/__init__.py:10:1: H306  imports not in alphabetical order (sqlalchemy.ext.declarative.declarative_base, sqlalchemy.column)
webcompat/db/__init__.py:12:1: H306  imports not in alphabetical order (sqlalchemy.string, sqlalchemy.orm.scoped_session)
webcompat/db/__init__.py:14:1: H306  imports not in alphabetical order (sqlalchemy.orm.sessionmaker, hashlib.sha512)
webcompat/error_handlers/__init__.py:76:7: H101  Use TODO(NAME)
webcompat/error_handlers/__init__.py:77:7: H101  Use TODO(NAME)
webcompat/webhooks/__init__.py:18:1: H306  imports not in alphabetical order (helpers.is_github_hook, helpers.get_issue_info)
2       H101  Use TODO(NAME)
11      H306  imports not in alphabetical order (flask_wtf.flaskform, flask_wtf.file.fileallowed)
5       H403  multi line docstrings should end on a new line
1       H405  multi line docstring summary not separated with an empty line

flake8 with hacking (aka the Openstack style we are using) installed.

[karlcow]

Documenting my thoughts, context.

In Our project we currently have in requirements.txt

• pep8==1.7.0 (which is the latest version)

On my own computer I have

• flake8==2.5.5 (which I installed with pip install --user flake8)
• hacking==0.13.0 (pip install --user hacking)
• flake8-docstrings==1.1.0 (pip install --user flake8-docstrings) The original request of this issue
• pyflakes==0.8.1 (checks for things like declared import but not used.)

Example of pyflakes.

pyflakes webcompat/
webcompat/__init__.py:28: 'webcompat' imported but unused
webcompat/form.py:12: 'os' imported but unused
webcompat/form.py:74: redefinition of unused 'os' from line 12
webcompat/feed/feed.py:40: undefined name 'is_known_domain'
webcompat/feed/feed.py:44: undefined name 'create_feed'
webcompat/feed/feed.py:46: undefined name 'serve_domain_feed'

In Sublime

• SublimeLinter
• sublimelinter-flake8

Examples of what I get

I configured my linting on Save/Load instead of Background to save CPU. So when I save I get this:

This gives a list of errors. clicking on the error brings you to the line with the error. (it's possible to deactivate the pop-up and just use the gutter and code highlighting)

Here

1. line 9: multiple import on one line.
2. line 14: import os not at the right place.
3. line 38: except written in the old style, and will break if/when switch to python 3

some issues

• People might have their own configuration of sublimelinter for other projects. We should probably document a project based configuration file. I need to test if http://www.sublimelinter.com/en/latest/settings.html#sublimelinterrc-settings would work in our project.

What needs to be done.

• install the python modules: pep8, flake8, hacking (should we include them in the requirements.txt. I don't even know if pep8 should be there. Maybe.)
• Install SublimeLinter through Packages Control
• install sublimelinter-flake8 through Packages Control

if I do flake8 with the above configurations on our current code. I get

→ flake8 webcompat/
webcompat/form.py:1:1: D205 1 blank line required between summary line and description
webcompat/form.py:1:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/form.py:1:1: D400 First line should end with a period
webcompat/form.py:17:1: H306  imports not in alphabetical order (flask_wtf.flaskform, flask_wtf.file.fileallowed)
webcompat/form.py:27:1: H306  imports not in alphabetical order (webcompat.app, webcompat.api.uploads.upload)
webcompat/form.py:63:1: D300 Use """triple double quotes"""
webcompat/form.py:63:1: D400 First line should end with a period
webcompat/form.py:68:1: D204 1 blank line required after class docstring
webcompat/form.py:110:1: D401 First line should be in imperative mood
webcompat/form.py:121:1: D401 First line should be in imperative mood; try rephrasing
webcompat/form.py:136:1: D403 First word of the first line should be properly capitalized
webcompat/helpers.py:1:1: D100 Missing docstring in public module
webcompat/helpers.py:43:1: D300 Use """triple double quotes"""
webcompat/helpers.py:50:1: D300 Use """triple double quotes"""
webcompat/helpers.py:68:1: D300 Use """triple double quotes"""
webcompat/helpers.py:81:1: D300 Use """triple double quotes"""
webcompat/helpers.py:92:1: D300 Use """triple double quotes"""
webcompat/helpers.py:102:1: D300 Use """triple double quotes"""
webcompat/helpers.py:121:1: D300 Use """triple double quotes"""
webcompat/helpers.py:134:1: D300 Use """triple double quotes"""
webcompat/helpers.py:157:1: D300 Use """triple double quotes"""
webcompat/helpers.py:170:1: D300 Use """triple double quotes"""
webcompat/helpers.py:188:1: D300 Use """triple double quotes"""
webcompat/helpers.py:203:1: D300 Use """triple double quotes"""
webcompat/helpers.py:217:1: D300 Use """triple double quotes"""
webcompat/helpers.py:233:1: D300 Use """triple double quotes"""
webcompat/helpers.py:233:1: D401 First line should be in imperative mood; try rephrasing
webcompat/helpers.py:245:1: D300 Use """triple double quotes"""
webcompat/helpers.py:245:1: D400 First line should end with a period
webcompat/helpers.py:275:1: D300 Use """triple double quotes"""
webcompat/helpers.py:303:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/helpers.py:303:1: D300 Use """triple double quotes"""
webcompat/helpers.py:306:28: H403  multi line docstrings should end on a new line
webcompat/helpers.py:313:1: D300 Use """triple double quotes"""
webcompat/helpers.py:328:1: D300 Use """triple double quotes"""
webcompat/helpers.py:333:1: D300 Use """triple double quotes"""
webcompat/helpers.py:356:1: D300 Use """triple double quotes"""
webcompat/helpers.py:363:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/helpers.py:363:1: D300 Use """triple double quotes"""
webcompat/helpers.py:363:1: D401 First line should be in imperative mood
webcompat/helpers.py:367:32: H403  multi line docstrings should end on a new line
webcompat/helpers.py:372:1: D300 Use """triple double quotes"""
webcompat/helpers.py:383:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/helpers.py:383:1: D300 Use """triple double quotes"""
webcompat/helpers.py:383:1: D400 First line should end with a period
webcompat/helpers.py:383:1: D401 First line should be in imperative mood
webcompat/helpers.py:388:15: H403  multi line docstrings should end on a new line
webcompat/helpers.py:411:1: D300 Use """triple double quotes"""
webcompat/helpers.py:428:1: D300 Use """triple double quotes"""
webcompat/helpers.py:450:1: D300 Use """triple double quotes"""
webcompat/helpers.py:450:1: D401 First line should be in imperative mood; try rephrasing
webcompat/helpers.py:474:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/helpers.py:474:1: D300 Use """triple double quotes"""
webcompat/helpers.py:478:28: H403  multi line docstrings should end on a new line
webcompat/helpers.py:486:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/helpers.py:486:1: D300 Use """triple double quotes"""
webcompat/helpers.py:490:28: H403  multi line docstrings should end on a new line
webcompat/helpers.py:504:1: D300 Use """triple double quotes"""
webcompat/helpers.py:504:1: D401 First line should be in imperative mood
webcompat/issues.py:1:1: D205 1 blank line required between summary line and description
webcompat/issues.py:1:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/issues.py:1:1: D300 Use """triple double quotes"""
webcompat/issues.py:1:1: D400 First line should end with a period
webcompat/issues.py:13:1: H306  imports not in alphabetical order (webcompat.github, webcompat.form.build_formdata)
webcompat/issues.py:18:1: D300 Use """triple double quotes"""
webcompat/views.py:1:1: D100 Missing docstring in public module
webcompat/views.py:22:1: H306  imports not in alphabetical order (form.proxy_report, form.get_form)
webcompat/views.py:33:1: H306  imports not in alphabetical order (webcompat.github, webcompat.db.user)
webcompat/views.py:34:1: H306  imports not in alphabetical order (webcompat.db.user, webcompat.db.session_db)
webcompat/views.py:38:1: D103 Missing docstring in public function
webcompat/views.py:43:1: D103 Missing docstring in public function
webcompat/views.py:52:1: D103 Missing docstring in public function
webcompat/views.py:60:1: D103 Missing docstring in public function
webcompat/views.py:74:1: D103 Missing docstring in public function
webcompat/views.py:90:1: D103 Missing docstring in public function
webcompat/views.py:100:1: D103 Missing docstring in public function
webcompat/views.py:132:1: D401 First line should be in imperative mood; try rephrasing
webcompat/views.py:239:1: D401 First line should be in imperative mood; try rephrasing
webcompat/views.py:248:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/__init__.py:1:1: D104 Missing docstring in public package
webcompat/api/endpoints.py:1:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/api/endpoints.py:1:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:1:1: D400 First line should end with a period
webcompat/api/endpoints.py:23:1: H306  imports not in alphabetical order (webcompat.limiter, webcompat.helpers.api_request)
webcompat/api/endpoints.py:38:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:49:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:70:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:70:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/endpoints.py:89:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:89:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/endpoints.py:110:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:153:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:183:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:214:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:230:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:248:1: D200 One-line docstring should fit on one line with quotes
webcompat/api/endpoints.py:248:1: D300 Use """triple double quotes"""
webcompat/api/endpoints.py:250:1: H405  multi line docstring summary not separated with an empty line
webcompat/api/uploads.py:1:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:31:1: D204 1 blank line required after class docstring
webcompat/api/uploads.py:31:1: D211 No blank lines allowed before class docstring
webcompat/api/uploads.py:31:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:41:1: D102 Missing docstring in public method
webcompat/api/uploads.py:54:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:54:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/uploads.py:71:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:79:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:79:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/uploads.py:88:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:88:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/uploads.py:92:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:92:1: D401 First line should be in imperative mood; try rephrasing
webcompat/api/uploads.py:96:1: D300 Use """triple double quotes"""
webcompat/api/uploads.py:125:1: D300 Use """triple double quotes"""
webcompat/db/__init__.py:1:1: D104 Missing docstring in public package
webcompat/db/__init__.py:10:1: H306  imports not in alphabetical order (sqlalchemy.ext.declarative.declarative_base, sqlalchemy.column)
webcompat/db/__init__.py:12:1: H306  imports not in alphabetical order (sqlalchemy.string, sqlalchemy.orm.scoped_session)
webcompat/db/__init__.py:14:1: H306  imports not in alphabetical order (sqlalchemy.orm.sessionmaker, hashlib.sha512)
webcompat/db/__init__.py:34:1: D101 Missing docstring in public class
webcompat/db/__init__.py:42:1: D102 Missing docstring in public method
webcompat/db/__init__.py:54:1: D101 Missing docstring in public class
webcompat/db/__init__.py:60:1: D102 Missing docstring in public method
webcompat/error_handlers/__init__.py:1:1: D300 Use """triple double quotes"""
webcompat/error_handlers/__init__.py:29:1: D300 Use """triple double quotes"""
webcompat/error_handlers/__init__.py:29:1: D401 First line should be in imperative mood; try rephrasing
webcompat/error_handlers/__init__.py:39:1: D300 Use """triple double quotes"""
webcompat/error_handlers/__init__.py:39:1: D400 First line should end with a period
webcompat/error_handlers/__init__.py:39:1: D401 First line should be in imperative mood; try rephrasing
webcompat/error_handlers/__init__.py:54:1: D103 Missing docstring in public function
webcompat/error_handlers/__init__.py:68:1: D300 Use """triple double quotes"""
webcompat/error_handlers/__init__.py:76:7: H101  Use TODO(NAME)
webcompat/error_handlers/__init__.py:77:7: H101  Use TODO(NAME)
webcompat/feed/__init__.py:1:1: D104 Missing docstring in public package
webcompat/feed/feed.py:40:8: F821 undefined name 'is_known_domain'
webcompat/feed/feed.py:44:13: F821 undefined name 'create_feed'
webcompat/feed/feed.py:46:16: F821 undefined name 'serve_domain_feed'
webcompat/feed/helpers.py:14:1: D401 First line should be in imperative mood
webcompat/feed/helpers.py:22:1: D401 First line should be in imperative mood
webcompat/feed/helpers.py:22:1: E302 expected 2 blank lines, found 1
webcompat/webhooks/__init__.py:1:1: D209 Multi-line docstring closing quotes should be on a separate line
webcompat/webhooks/__init__.py:1:1: D400 First line should end with a period
webcompat/webhooks/__init__.py:18:1: H306  imports not in alphabetical order (helpers.is_github_hook, helpers.get_issue_info)
webcompat/webhooks/helpers.py:1:1: D100 Missing docstring in public module
webcompat/webhooks/helpers.py:16:1: D400 First line should end with a period
webcompat/webhooks/helpers.py:36:1: D401 First line should be in imperative mood; try rephrasing
webcompat/webhooks/helpers.py:52:1: D402 First line should not be the function's "signature"
webcompat/webhooks/helpers.py:64:1: D401 First line should be in imperative mood
webcompat/webhooks/helpers.py:104:1: D300 Use """triple double quotes"""

I DO NOT WANT us to make a massive commit with all the fixes. I want us to fix code when we are working on a specific module which needs these fixes. This to avoid to break the history. Most of these fixes are just syntactic. The code is working.

It helps me a lot when I do code review for example and when I write code. It's directly visible.

The default settings for these checker are fine with me.

I don't know yet if we should enforce this in #1661 I'm always a bit worried with blocking people to commit until their code is perfectly syntactically correct. I do that for myself usually, though I fail sometimes. I'm not convinced that enforcing my hygiene on others is desirable, specifically for early contributors. It creates also the possibility of a dialogue during code review.

@miketaylr @etsai @brizental other python regular coders on the project. Any ideas, comments?

[miketaylr]

+1.

I like the idea of fixing things as we touch those files. I also like the idea of having a standard set of linting packages/settings -- can we document this in CONTRIBUTING.md or the wiki? I think it's fine to document how to work with SublimeText, and others can contribute config docs for other editors if they care.

[miketaylr]

I propose @karlcow @MDTsai and I tackle this next week in Taipei.