Misc lint cleanup

Cleanup a bunch of lint issues. Still have three files with lint issues:
repository_git, sourcetree, test_unit_externals_description.

Testing:
  python2 unit tests - pass
  python2 clm manual testing checkout, status - ok
  python2 cesm manual testing checkout, status - ok
  python3 unit tests - 3 errors

Author: bjandre

checkout_externals.py

@@ -14,11 +14,15 @@
 import logging
 import os
 import os.path
-import re
 import sys
 import textwrap
 import traceback
 
+from manic import read_externals_description_file, create_externals_description
+from manic import SourceTree
+from manic import check_safe_to_update_repos
+from manic import printlog, PPRINTER
+
 if sys.hexversion < 0x02070000:
     print(70 * '*')
     print('ERROR: {0} requires python >= 2.7.x. '.format(sys.argv[0]))
@@ -27,11 +31,6 @@
     print(70 * '*')
     sys.exit(1)
 
-from manic import read_externals_description_file, create_externals_description
-from manic import SourceTree
-from manic import check_safe_to_update_repos
-from manic import printlog, PPRINTER
-
 
 # ---------------------------------------------------------------------
 #
@@ -244,7 +243,7 @@ def _main(args):
 
     if args.status:
         # user requested status-only
-        for comp in sorted(tree_status.iterkeys()):
+        for comp in sorted(tree_status.keys()):
             msg = str(tree_status[comp])
             printlog(msg)
         if args.verbose:
@@ -255,7 +254,7 @@ def _main(args):
         safe_to_update = check_safe_to_update_repos(tree_status, args.debug)
         if not safe_to_update:
             # print status
-            for comp in sorted(tree_status.iterkeys()):
+            for comp in sorted(tree_status.keys()):
                 msg = str(tree_status[comp])
                 printlog(msg)
             # exit gracefully

manic/externals_description.py

@@ -28,15 +28,24 @@
 try:
     # python2
     from ConfigParser import SafeConfigParser as config_parser
-    import ConfigParser
+    from ConfigParser import MissingSectionHeaderError
+    from ConfigParser import NoSectionError, NoOptionError
 
     def config_string_cleaner(text):
+        """convert strings into unicode
+        """
         return text.decode('utf-8')
 except ImportError:
     # python3
     from configparser import ConfigParser as config_parser
+    from configparser import MissingSectionHeaderError
+    from configparser import NoSectionError, NoOptionError
 
     def config_string_cleaner(text):
+        """Python3 already uses unicode strings, so just return the string
+        without modification.
+
+        """
         return text
 
 from .utils import printlog, fatal_error, str_to_bool
@@ -70,7 +79,7 @@ def read_externals_description_file(root_dir, file_name):
         config = config_parser()
         config.read(file_path)
         externals_description = config
-    except ConfigParser.MissingSectionHeaderError:
+    except MissingSectionHeaderError:
         # not a cfg file
         pass
 
@@ -115,8 +124,8 @@ def get_cfg_schema_version(model_cfg):
     semver_str = ''
     try:
         semver_str = model_cfg.get(DESCRIPTION_SECTION, VERSION_ITEM)
-    except Exception:
-        msg = ('ERROR: externals description file must have the required '
+    except (NoSectionError, NoOptionError):
+        msg = ('externals description file must have the required '
                'section: "{0}" and item "{1}"'.format(DESCRIPTION_SECTION,
                                                       VERSION_ITEM))
         fatal_error(msg)
@@ -291,6 +300,7 @@ class ExternalsDescriptionDict(ExternalsDescription):
     description files for unit testing.
 
     """
+
     def __init__(self, model_data):
         """Parse a native dictionary into a externals description.
         """
@@ -304,6 +314,7 @@ class ExternalsDescriptionConfigV1(ExternalsDescription):
     schema version 1.
 
     """
+
     def __init__(self, model_data):
         """Convert the xml into a standardized dict that can be used to
         construct the source objects
@@ -350,5 +361,3 @@ def list_to_dict(input_list, convert_to_lower_case=True):
                 if item in self._source_schema[self.REPO]:
                     self[name][self.REPO][item] = self[name][item]
                     del self[name][item]
-
-

manic/externalstatus.py

@@ -1,3 +1,13 @@
+"""ExternalStatus
+
+Class to store status and state information about repositories and
+create a string representation.
+
+"""
+from __future__ import absolute_import
+from __future__ import unicode_literals
+from __future__ import print_function
+
 from .globals import EMPTY_STR
 from .utils import printlog
 

manic/repository_factory.py

@@ -1,3 +1,10 @@
+"""Factory for creating and initializing the appropriate repository class
+"""
+
+from __future__ import absolute_import
+from __future__ import unicode_literals
+from __future__ import print_function
+
 from .repository_git import GitRepository
 from .repository_svn import SvnRepository
 from .externals_description import ExternalsDescription

manic/repository_git.py

@@ -1,3 +1,10 @@
+"""Class for interacting with git repositories
+"""
+
+from __future__ import absolute_import
+from __future__ import unicode_literals
+from __future__ import print_function
+
 import os
 import re
 
@@ -315,23 +322,7 @@ def _git_checkout(self, repo_dir_path):
 
         cmd = []
         if self._branch:
-            (curr_branch, _) = self._git_current_branch()
-            ref_type = self._git_ref_type(self._branch)
-            if ref_type == self.GIT_REF_REMOTE_BRANCH:
-                cmd = ['git', 'checkout', '--track', 'origin/' + self._branch]
-            elif ref_type == self.GIT_REF_LOCAL_BRANCH:
-                if curr_branch != self._branch:
-                    if not self._git_working_dir_clean(repo_dir_path):
-                        msg = ('Working directory "{0}" not clean, '
-                               'aborting'.format(repo_dir_path))
-                        fatal_error(msg)
-                    else:
-                        cmd = ['git', 'checkout', self._branch]
-
-            else:
-                msg = 'Unable to check out branch, "{0}"'.format(self._branch)
-                fatal_error(msg)
-
+            cmd = self._checkout_branch_command(repo_dir_path)
         elif self._tag:
             # For now, do a hail mary and hope tag can be checked out
             cmd = ['git', 'checkout', self._tag]
@@ -344,6 +335,31 @@ def _git_checkout(self, repo_dir_path):
 
         os.chdir(cwd)
 
+    def _checkout_branch_command(self, repo_dir_path):
+        """Construct the command for checking out the specified branch
+        """
+        cmd = []
+        (curr_branch, _) = self._git_current_branch()
+        ref_type = self._git_ref_type(self._branch)
+        if ref_type == self.GIT_REF_REMOTE_BRANCH:
+            cmd = ['git', 'checkout', '--track', 'origin/' + self._branch]
+        elif ref_type == self.GIT_REF_LOCAL_BRANCH:
+            if curr_branch != self._branch:
+                # FIXME(bja, 2017-11) not sure what this branch logic
+                # is accomplishing, but it can lead to cmd being
+                # undefined without an error. Probably not what we
+                # want!
+                if not self._git_working_dir_clean(repo_dir_path):
+                    msg = ('Working directory "{0}" not clean, '
+                           'aborting'.format(repo_dir_path))
+                    fatal_error(msg)
+                else:
+                    cmd = ['git', 'checkout', self._branch]
+        else:
+            msg = 'Unable to check out branch, "{0}"'.format(self._branch)
+            fatal_error(msg)
+        return cmd
+
     @staticmethod
     def git_status_porcelain_v1z():
         """Run the git status command on the cwd and report results in the

manic/repository_svn.py

@@ -1,3 +1,10 @@
+"""Class for interacting with svn repositories
+"""
+
+from __future__ import absolute_import
+from __future__ import unicode_literals
+from __future__ import print_function
+
 import logging
 import os
 import re

manic/sourcetree.py

@@ -196,7 +196,7 @@ def _create_externals_sourcetree(self):
 
         externals_root = self._repo_dir_path
         model_data = read_externals_description_file(externals_root,
-                                                 self._externals)
+                                                     self._externals)
         externals = create_externals_description(model_data)
         self._externals_sourcetree = SourceTree(externals_root, externals)
         os.chdir(cwd)
@@ -237,16 +237,16 @@ def status(self, relative_path_base='.'):
         for comp in load_comps:
             printlog('{0}, '.format(comp), end='')
             stat = self._all_components[comp].status()
-            for comp in stat.keys():
+            for name in stat.keys():
                 # check if we need to append the relative_path_base to
                 # the path so it will be sorted in the correct order.
-                if not stat[comp].path.startswith(relative_path_base):
-                    stat[comp].path = os.path.join(relative_path_base,
-                                                   stat[comp].path)
+                if not stat[name].path.startswith(relative_path_base):
+                    stat[name].path = os.path.join(relative_path_base,
+                                                   stat[name].path)
                     # store under key = updated path, and delete the
                     # old key.
-                    comp_stat = stat[comp]
-                    del stat[comp]
+                    comp_stat = stat[name]
+                    del stat[name]
                     stat[comp_stat.path] = comp_stat
             summary.update(stat)
 

manic/utils.py

@@ -67,9 +67,10 @@ def str_to_bool(bool_str):
     Conversion should be case insensitive.
     """
     value = None
-    if ((bool_str.lower() == 'true') or (bool_str.lower() == 't')):
+    str_lower = bool_str.lower()
+    if (str_lower == 'true') or (str_lower == 't'):
         value = True
-    elif bool_str.lower() == 'false' or bool_str.lower() == 'f':
+    elif (str_lower == 'false') or (str_lower == 'f'):
         value = False
     if value is None:
         msg = ('ERROR: invalid boolean string value "{0}". '

test/Makefile

@@ -22,7 +22,7 @@ COVERAGE_ARGS=--rcfile=.coveragerc
 
 SRC = \
 	../checkout_externals.py \
-	../manageexternals/*.py
+	../manic/*.py
 
 EXE = ../checkout_externals.py
 

test/test_unit_externals_description.py

@@ -11,21 +11,25 @@
 from __future__ import unicode_literals
 from __future__ import print_function
 
-import string
-import sys
 import unittest
 
 try:
     # python2
     from ConfigParser import SafeConfigParser as config_parser
 
     def config_string_cleaner(text):
+        """convert strings into unicode
+        """
         return text.decode('utf-8')
 except ImportError:
     # python3
     from configparser import ConfigParser as config_parser
 
     def config_string_cleaner(text):
+        """Python3 already uses unicode strings, so just return the string
+        without modification.
+
+        """
         return text
 
 from manic.externals_description import DESCRIPTION_SECTION, VERSION_ITEM