flake8: enforce import order and adopt W504

* Add flake8-import-order to enforce the import order using the 'edited'
  style that corresponds to our styleguide, see:
  https://www.mediawiki.org/wiki/Manual:Coding_conventions/Python#Imports
* Fix all out of order imports.
* For line breaks around binary operators, adopt W504 (breaking before
  the operator) and ignore W503, following PEP8 suggestion, see:
  https://www.python.org/dev/peps/pep-0008/#should-a-line-break-before-or-after-a-binary-operator
* Fix all line breaks around binary operators to follow W504

Change-Id: Id4fe4b5a6b613df1e85e6e7ce31f40755d21ec7b

cumin/__init__.py

@@ -2,11 +2,10 @@
 import logging
 import os
 
-from pkg_resources import DistributionNotFound, get_distribution
-
 import yaml
 
 from ClusterShell.NodeSet import NodeSet, RESOLVER_NOGROUP
+from pkg_resources import DistributionNotFound, get_distribution
 
 
 try:
@@ -30,8 +29,8 @@ class CuminError(Exception):
 # Fail if the custom logging slot is already in use with a different name or
 # Access to a private property of logging was preferred over matching the default string returned by
 # logging.getLevelName() for unused custom slots.
-if (LOGGING_TRACE_LEVEL_NUMBER in logging._levelToName and  # pylint: disable=protected-access
-        LOGGING_TRACE_LEVEL_NAME not in logging._nameToLevel):  # pylint: disable=protected-access
+if (LOGGING_TRACE_LEVEL_NUMBER in logging._levelToName  # pylint: disable=protected-access
+        and LOGGING_TRACE_LEVEL_NAME not in logging._nameToLevel):  # pylint: disable=protected-access
     raise CuminError("Unable to set custom logging for trace, logging level {level} is alredy set for '{name}'.".format(
         level=LOGGING_TRACE_LEVEL_NUMBER, name=logging.getLevelName(LOGGING_TRACE_LEVEL_NUMBER)))
 

cumin/backends/direct.py

@@ -22,8 +22,8 @@ def grammar():
 
     """
     # Boolean operators
-    boolean = (pp.CaselessKeyword('and not').leaveWhitespace() | pp.CaselessKeyword('and') |
-               pp.CaselessKeyword('xor') | pp.CaselessKeyword('or'))('bool')
+    boolean = (pp.CaselessKeyword('and not').leaveWhitespace() | pp.CaselessKeyword('and')
+               | pp.CaselessKeyword('xor') | pp.CaselessKeyword('or'))('bool')
 
     # Parentheses
     lpar = pp.Literal('(')('open_subgroup')

cumin/backends/knownhosts.py

@@ -33,8 +33,8 @@ def grammar():
 
     """
     # Boolean operators
-    boolean = (pp.CaselessKeyword('and not').leaveWhitespace() | pp.CaselessKeyword('and') |
-               pp.CaselessKeyword('xor') | pp.CaselessKeyword('or'))('bool')
+    boolean = (pp.CaselessKeyword('and not').leaveWhitespace() | pp.CaselessKeyword('and')
+               | pp.CaselessKeyword('xor') | pp.CaselessKeyword('or'))('bool')
 
     # Parentheses
     lpar = pp.Literal('(')('open_subgroup')

cumin/grammar.py

@@ -80,8 +80,8 @@ def grammar(backend_keys):
 
     """
     # Boolean operators
-    boolean = (pp.CaselessKeyword('and not').leaveWhitespace() | pp.CaselessKeyword('and') |
-               pp.CaselessKeyword('xor') | pp.CaselessKeyword('or'))('bool')
+    boolean = (pp.CaselessKeyword('and not').leaveWhitespace() | pp.CaselessKeyword('and')
+               | pp.CaselessKeyword('xor') | pp.CaselessKeyword('or'))('bool')
 
     # Parentheses
     lpar = pp.Literal('(')('open_subgroup')

cumin/tests/integration/test_cli.py

@@ -147,8 +147,8 @@ def test_variant(self, capsys):
             mode = 'async'
         else:
             mode = 'sync'
-            labels += (get_ls_expected_lines(params) + get_date_expected_lines(params) +
-                       get_timeout_expected_lines(params))
+            labels += (get_ls_expected_lines(params) + get_date_expected_lines(params)
+                       + get_timeout_expected_lines(params))
 
         for label in labels:
             if label in ('all_success', 'all_failure') and '-p' in params['params']:

cumin/tests/unit/transports/test_clustershell.py

@@ -5,7 +5,7 @@
 import pytest
 
 from cumin import CuminError, nodeset
-from cumin.transports import BaseWorker, Command, clustershell, ProgressBars, State, Target, WorkerError
+from cumin.transports import BaseWorker, clustershell, Command, ProgressBars, State, Target, WorkerError
 
 
 def test_node_class_instantiation():

cumin/transports/__init__.py

@@ -652,8 +652,8 @@ def success_threshold(self):
     @success_threshold.setter
     def success_threshold(self, value):
         """Setter for the `success_threshold` property. The relative documentation is in the getter."""
-        if value is not None and (not isinstance(value, float) or
-                                  not (0.0 <= value <= 1.0)):  # pylint: disable=superfluous-parens
+        if value is not None and (not isinstance(value, float)
+                                  or not (0.0 <= value <= 1.0)):  # pylint: disable=superfluous-parens
             raise WorkerError("success_threshold must be a float beween 0 and 1, got '{value_type}': {value}".format(
                 value_type=type(value), value=value))
 

cumin/transports/clustershell.py

@@ -221,8 +221,8 @@ def on_timeout(self, task):
             # batches) and those that were already scheduled (for example when the # of nodes is greater than
             # ClusterShell fanout)
             pending_or_scheduled = sum(
-                (node.state.is_pending or node.state.is_scheduled or
-                 (node.state.is_success and node.running_command_index < (len(node.commands) - 1))
+                (node.state.is_pending or node.state.is_scheduled
+                 or (node.state.is_success and node.running_command_index < (len(node.commands) - 1))
                  ) for node in self.nodes.values())
             if pending_or_scheduled > 0:
                 self.progress.update_failed(num_timeout + pending_or_scheduled)
@@ -424,8 +424,8 @@ def _success_nodes_report(self, command=None):
             command (str, optional): the command the report is referring to.
         """
         if self.global_timedout and command is None:
-            num = sum(1 for node in self.nodes.values() if node.state.is_success and
-                      node.running_command_index == (len(self.commands) - 1))
+            num = sum(1 for node in self.nodes.values() if node.state.is_success
+                      and node.running_command_index == (len(self.commands) - 1))
         else:
             num = self.counters['success']
 

doc/source/conf.py

@@ -22,10 +22,9 @@
 import sys
 import types
 
-from pkg_resources import get_distribution
-
 import sphinx_rtd_theme
 
+from pkg_resources import get_distribution
 from sphinx import __version__ as sphinx_version
 
 # Adjust path
@@ -166,8 +165,8 @@ def filter_namedtuple_docstrings(app, what, name, obj, options, lines):
 
 def add_abstract_annotations(app, what, name, obj, options, lines):
     """Workaround to add an abstract annotation for ABC abstract classes and abstract methods."""
-    if ((what in ('method', 'attribute') and getattr(obj, '__isabstractmethod__', False)) or
-            (what == 'class' and len(getattr(obj, '__abstractmethods__', [])) > 0)):
+    if ((what in ('method', 'attribute') and getattr(obj, '__isabstractmethod__', False))
+            or (what == 'class' and len(getattr(obj, '__abstractmethods__', [])) > 0)):
         lines.insert(0, '``abstract``')
 
 

setup.py

@@ -31,6 +31,7 @@
     'tests': [
         'bandit>=1.1.0,<1.6.0',
         'flake8>=3.2.1',
+        'flake8-import-order>=0.18.1',
         'prospector[with_everything]>=0.12.4',
         'pytest-cov>=1.8.0',
         'pytest-xdist>=1.15.0',

tox.ini

@@ -62,3 +62,9 @@ setenv =
 [flake8]
 max-line-length = 120
 statistics = True
+ignore = W503
+import-order-style = edited
+# Do not specify application-package-names to avoid to manually keep the list of Org-wide packages
+# application-package-names =
+# Mark cumin as local to separate its imports
+application-import-names = cumin