Merge pull request #1353 from epfl-scitas/qa/minor_cleanup

qa : minor cleanup

2 files changed, 39 insertions, 53 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 8bdae0445e..c6277fc8d2 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -95,32 +95,30 @@ thing.  Spack uses ~variant in directory names and in the canonical form of
 specs to avoid ambiguity.  Both are provided because ~ can cause shell
 expansion when it is the first character in an id typed on the command line.
 """
-import sys
-import hashlib
 import base64
+import hashlib
 import imp
+import sys
 from StringIO import StringIO
 from operator import attrgetter
-import yaml
-from yaml.error import MarkedYAMLError
 
 import llnl.util.tty as tty
-from llnl.util.lang import *
-from llnl.util.tty.color import *
-from llnl.util.filesystem import join_path
-
 import spack
 import spack.architecture
-import spack.parse
-import spack.error
 import spack.compilers as compilers
-
-from spack.version import *
-from spack.util.string import *
-from spack.util.prefix import Prefix
+import spack.error
+import spack.parse
+import yaml
+from llnl.util.filesystem import join_path
+from llnl.util.lang import *
+from llnl.util.tty.color import *
+from spack.build_environment import get_path_from_module, load_module
 from spack.util.naming import mod_to_class
+from spack.util.prefix import Prefix
+from spack.util.string import *
+from spack.version import *
 from spack.virtual import ProviderIndex
-from spack.build_environment import get_path_from_module, load_module
+from yaml.error import MarkedYAMLError
 
 # Valid pattern for an identifier in Spack
 identifier_re = r'\w[\w-]*'
@@ -161,19 +159,6 @@ special_types = {
 }
 
 
-def index_specs(specs):
-    """Take a list of specs and return a dict of lists.  Dict is
-       keyed by spec name and lists include all specs with the
-       same name.
-    """
-    spec_dict = {}
-    for spec in specs:
-        if spec.name not in spec_dict:
-            spec_dict[spec.name] = []
-        spec_dict[spec.name].append(spec)
-    return spec_dict
-
-
 def colorize_spec(spec):
     """Returns a spec colorized according to the colors specified in
        color_formats."""
@@ -1036,7 +1021,7 @@ class Spec(object):
         """
         try:
             yfile = yaml.load(stream)
-        except MarkedYAMLError, e:
+        except MarkedYAMLError as e:
             raise SpackYAMLError("error parsing YAML spec:", str(e))
 
         nodes = yfile['spec']
@@ -1350,7 +1335,7 @@ class Spec(object):
 
             return flat_deps
 
-        except UnsatisfiableSpecError, e:
+        except UnsatisfiableSpecError as e:
             # Here, the DAG contains two instances of the same package
             # with inconsistent constraints.  Users cannot produce
             # inconsistent specs like this on the command line: the
@@ -1385,7 +1370,7 @@ class Spec(object):
                     dep = Spec(name)
                 try:
                     dep.constrain(dep_spec)
-                except UnsatisfiableSpecError, e:
+                except UnsatisfiableSpecError as e:
                     e.message = ("Conflicting conditional dependencies on"
                                  "package %s for spec %s" % (self.name, self))
                     raise e
@@ -1471,7 +1456,7 @@ class Spec(object):
         try:
             changed |= spec_deps[dep.name].spec.constrain(dep)
 
-        except UnsatisfiableSpecError, e:
+        except UnsatisfiableSpecError as e:
             e.message = "Invalid spec: '%s'. "
             e.message += "Package %s requires %s %s, but spec asked for %s"
             e.message %= (spec_deps[dep.name].spec, dep.name,
@@ -2405,7 +2390,7 @@ class SpecParser(spack.parse.Parser):
                     # errors now?
                     specs.append(self.spec(None, True))
 
-        except spack.parse.ParseError, e:
+        except spack.parse.ParseError as e:
             raise SpecParseError(e)
 
         # If the spec has an os or a target and no platform, give it
@@ -2850,4 +2835,4 @@ class AmbiguousHashError(SpecError):
     def __init__(self, msg, *specs):
         super(AmbiguousHashError, self).__init__(msg)
         for spec in specs:
-            print '    ', spec.format('$.$@$%@+$+$=$#')
+            print('    ', spec.format('$.$@$%@+$+$=$#'))
diff --git a/lib/spack/spack/test/cmd/test_install.py b/lib/spack/spack/test/cmd/test_install.py
index a94d3c8bba..39287d5d6d 100644
--- a/lib/spack/spack/test/cmd/test_install.py
+++ b/lib/spack/spack/test/cmd/test_install.py
@@ -22,18 +22,24 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
+import StringIO
 import collections
-from contextlib import contextmanager
+import os
+import unittest
+import contextlib
 
-import StringIO
+import spack
+import spack.cmd
 
 FILE_REGISTRY = collections.defaultdict(StringIO.StringIO)
 
+
 # Monkey-patch open to write module files to a StringIO instance
-@contextmanager
+@contextlib.contextmanager
 def mock_open(filename, mode):
     if not mode == 'wb':
-        raise RuntimeError('test.test_install : unexpected opening mode for monkey-patched open')
+        message = 'test.test_install : unexpected opening mode for mock_open'
+        raise RuntimeError(message)
 
     FILE_REGISTRY[filename] = StringIO.StringIO()
 
@@ -44,19 +50,14 @@ def mock_open(filename, mode):
         FILE_REGISTRY[filename] = handle.getvalue()
         handle.close()
 
-import os
-import itertools
-import unittest
-
-import spack
-import spack.cmd
 
-
-# The use of __import__ is necessary to maintain a name with hyphen (which cannot be an identifier in python)
+# The use of __import__ is necessary to maintain a name with hyphen (which
+# cannot be an identifier in python)
 test_install = __import__("spack.cmd.test-install", fromlist=['test_install'])
 
 
 class MockSpec(object):
+
     def __init__(self, name, version, hashStr=None):
         self._dependencies = {}
         self.name = name
@@ -88,10 +89,6 @@ class MockSpec(object):
         for _, spec in self._dependencies.items():
             yield spec.spec
         yield self
-        #from_iterable = itertools.chain.from_iterable
-        #allDeps = from_iterable(i.traverse()
-        #                        for i in self.dependencies())
-        #return set(itertools.chain([self], allDeps))
 
     def dag_hash(self):
         return self.hash
@@ -102,6 +99,7 @@ class MockSpec(object):
 
 
 class MockPackage(object):
+
     def __init__(self, spec, buildLogPath):
         self.name = spec.name
         self.spec = spec
@@ -113,6 +111,7 @@ class MockPackage(object):
 
 
 class MockPackageDb(object):
+
     def __init__(self, init=None):
         self.specToPkg = {}
         if init:
@@ -133,6 +132,7 @@ pkgY = MockPackage(specY, 'logY')
 
 
 class MockArgs(object):
+
     def __init__(self, package):
         self.package = package
         self.jobs = None
@@ -168,7 +168,7 @@ class TestInstallTest(unittest.TestCase):
         test_install.open = mock_open
 
         # Clean FILE_REGISTRY
-        FILE_REGISTRY = collections.defaultdict(StringIO.StringIO)
+        FILE_REGISTRY.clear()
 
         pkgX.installed = False
         pkgY.installed = False
@@ -194,7 +194,7 @@ class TestInstallTest(unittest.TestCase):
         spack.repo = self.saved_db
 
     def test_installing_both(self):
-        test_install.test_install(None, MockArgs('X') )
+        test_install.test_install(None, MockArgs('X'))
         self.assertEqual(len(FILE_REGISTRY), 1)
         for _, content in FILE_REGISTRY.items():
             self.assertTrue('tests="2"' in content)
@@ -210,4 +210,5 @@ class TestInstallTest(unittest.TestCase):
             self.assertTrue('tests="2"' in content)
             self.assertTrue('failures="0"' in content)
             self.assertTrue('errors="0"' in content)
-            self.assertEqual(sum('skipped' in line for line in content.split('\n')), 2)
+            self.assertEqual(
+                sum('skipped' in line for line in content.split('\n')), 2)