Robottelo: PEP8 and Robottelo style guide

Created on 5 Jan 2017  路  5Comments  路  Source: SatelliteQE/robottelo

PEP8 and Robottelo's official docs state the same about import:

Imports should be grouped in the following order:

standard library imports
related third party imports
local application/library specific imports

You should put a blank line between each group of imports.

Besides that on reviews different style is required. In our unnoficial doc you can find style guide to import like this:

    import a  
    import b  
    import c  
    from d.a import a  
    from d.c import z  
    from z import a  

But once it is not stating groups of import, it is ambiguous.

My personal concern here is that automated tool like Pycharm and vim plugin follow strictly PEP8 so it reorganize import like this:

# PEP 8
import logging
from functools import wraps
from xml.parsers.expat import ExpatError, ErrorString

import bugzilla
import pytest
import re
import requests
import unittest2
from six.moves.xmlrpc_client import Fault

from robottelo.config import settings
from robottelo.constants import BZ_OPEN_STATUSES, NOT_IMPLEMENTED
from robottelo.host_info import get_host_sat_version

But once from imports are required to be together on reviews, one need to manually organize them:


import logging

import bugzilla
import pytest
import re
import requests
import unittest2
from six.moves.xmlrpc_client import Fault

from functools import wraps
from robottelo.config import settings
from robottelo.constants import BZ_OPEN_STATUSES, NOT_IMPLEMENTED
from robottelo.host_info import get_host_sat_version

Currently flake is not reinforcing import styling

So I see 2 options:

1) Follow PEP8, the upstream style guide so people,like me with Pycharm, can let automated tools take care of imports and turn on reinforcement on flake 8 if possible.

2) Keep import of kind "from" together, changing Robottelo docs accordingly and making people to organize import manually.

Obviously I vote on 1, but what you @SatelliteQE/quality-engineers think?.

Question

Most helpful comment

My vote is to use PEP8 standard style, then we can run autopep8 in our codebase to fix current style. Also we can include flake8-import-order plugin on travis-ci.

All 5 comments

My vote is to use PEP8 standard style, then we can run autopep8 in our codebase to fix current style. Also we can include flake8-import-order plugin on travis-ci.

+1 for automating the process.

-1 for automating the process , can lead to serious hidden problems
my opinion is that automation should not touch to code after it has been committed, perhaps I'm too conservative :)

@ldjebran on "automate the process" we mean using Ctrl + Alt + L in PyCharm for example, which automatic adjusts pep8 and import order. Or of course use another plugin for doing that in a different editor, vim python, emacs-python-mode and others can do that. Also there is auto-pep8 command line tool which can do that.

So if we move to use pep8 import style, we need to run the fix once, or maybe we just do that on demand.

But I really think it is better to use pep8 ordering.

@rochacbruno , thanks

+1 pep8 ordering (this is the best option) :)
+1 also for an automated refactoring (that should touch only the import ordering) that goes in it's own PR, think we can do it once with the script you mentioned.

Was this page helpful?
0 / 5 - 0 ratings