Merge pull request #34 from PamelaM/pm/add_site_packages_group

Add groups.SitePackagesGroup

AUTHORS.rst

@@ -10,3 +10,4 @@ Contributors
 ~~~~~~~~~~~~
 
 Benjamin Abel  - https://github.com/benjaminabel
+Pamela McA'Nulty - https://github.com/PamelaM

README.rst

@@ -73,6 +73,9 @@ Config file is simply a ``json`` file like this::
             {
                 "type": "stdlib"
             },
+            {
+                "type": "sitepackages"
+            },
             {
                 "type": "remainder"
             },
@@ -90,11 +93,14 @@ Config file is simply a ``json`` file like this::
 
 Default config looks something like::
 
-    {   
+    {
         "groups": [
             {
                 "type": "stdlib"
             },
+            {
+                "type": "sitepackages"
+            },
             {
                 "type": "remainder"
             },
@@ -110,6 +116,7 @@ to organize imports and will output import groups in the same order
 as defined in the config file. These are the supported group types:
 
 * ``stdlib`` - standard library imports including ``__future__``
+* ``sitepackages`` - imports coming from the ``site-packages`` directory
 * ``local`` - local imports which start with ``"."``. for example
   ``from .foo import bar``
 * ``packages`` - if this group is specified, additional key ``packages``

importanize/groups.py

@@ -8,7 +8,7 @@
 import six
 
 from .formatters import DEFAULT_FORMATTER
-from .utils import is_std_lib
+from .utils import is_std_lib, is_site_package
 
 
 @six.python_2_unicode_compatible
@@ -94,6 +94,11 @@ def should_add_statement(self, statement):
         return is_std_lib(statement.root_module)
 
 
+class SitePackagesGroup(BaseImportGroup):
+    def should_add_statement(self, statement):
+        return is_site_package(statement.root_module)
+
+
 class PackagesGroup(BaseImportGroup):
     def __init__(self, *args, **kwargs):
         super(PackagesGroup, self).__init__(*args, **kwargs)
@@ -116,9 +121,10 @@ class RemainderGroup(BaseImportGroup):
     def should_add_statement(self, statement):
         return True
 
-
+# -- RemainderGroup goes last and catches everything left over
 GROUP_MAPPING = OrderedDict((
     ('stdlib', StdLibGroup),
+    ('sitepackages', SitePackagesGroup),
     ('packages', PackagesGroup),
     ('local', LocalGroup),
     ('remainder', RemainderGroup),

importanize/main.py

@@ -29,6 +29,9 @@
         {
             'type': 'stdlib',
         },
+        {
+            'type': 'sitepackages',
+        },
         {
             'type': 'remainder',
         },

importanize/utils.py

@@ -10,40 +10,55 @@
 @contextmanager
 def ignore_site_packages_paths():
     paths = sys.path[:]
-    # remove working directory so that all
-    # local imports fail
-    if os.getcwd() in sys.path:
-        sys.path.remove(os.getcwd())
-    # remove all third-party paths
-    # so that only stdlib imports will succeed
-    sys.path = list(set(filter(
-        None,
-        filter(lambda i: all(('site-packages' not in i,
-                              'python' in i or 'pypy' in i)),
-               map(operator.methodcaller('lower'), sys.path))
-    )))
-    yield
-    sys.path = paths
-
-
-def is_std_lib(module):
-    if not module:
+    try:
+        # remove working directory so that all
+        # local imports fail
+        if os.getcwd() in sys.path:
+            sys.path.remove(os.getcwd())
+        # remove all third-party paths
+        # so that only stdlib imports will succeed
+        sys.path = list(set(filter(
+            None,
+            filter(lambda i: all(('site-packages' not in i,
+                                  'python' in i or 'pypy' in i)),
+                   map(operator.methodcaller('lower'), sys.path))
+        )))
+        yield
+    finally:
+        sys.path = paths
+
+
+def _safe_import_module(module_name):
+    imported_module = sys.modules.pop(module_name, None)
+    try:
+        return import_module(module_name)
+    except ImportError:
+        return None
+    finally:
+        if imported_module:
+            sys.modules[module_name] = imported_module
+
+
+def is_std_lib(module_name):
+    if not module_name:
         return False
 
-    if module in sys.builtin_module_names:
+    if module_name in sys.builtin_module_names:
         return True
 
     with ignore_site_packages_paths():
-        imported_module = sys.modules.pop(module, None)
-        try:
-            import_module(module)
-        except ImportError:
-            return False
-        else:
-            return True
-        finally:
-            if imported_module:
-                sys.modules[module] = imported_module
+        return bool(_safe_import_module(module_name))
+
+
+def is_site_package(module_name):
+    if not module_name:
+        return False
+
+    module = _safe_import_module(module_name)
+    module_path = getattr(module, "__file__", "")
+    if "site-packages" not in module_path:
+        return False
+    return "python" in module_path or "pypy" in module_path
 
 
 def list_strip(data):

tests/test_data/input.txt

@@ -14,6 +14,7 @@ from a.b import d
 
 import z
 from z import *
+import coverage  # in site-packages
 from z import foo
 
 import something # with comment

tests/test_data/output_grouped.txt

@@ -4,6 +4,8 @@ from __future__ import print_function, unicode_literals
 import datetime
 from os import path as ospath
 
+import coverage  # in site-packages
+
 import something  # with comment
 import z
 from a import b

tests/test_data/output_inline_grouped.txt

@@ -4,6 +4,8 @@ from __future__ import print_function, unicode_literals
 import datetime
 from os import path as ospath
 
+import coverage  # in site-packages
+
 import something  # with comment
 import z
 from a import b

tests/test_groups.py

@@ -12,6 +12,7 @@
     PackagesGroup,
     RemainderGroup,
     StdLibGroup,
+    SitePackagesGroup,
 )
 from importanize.statements import ImportLeaf, ImportStatement
 
@@ -184,6 +185,15 @@ def test_str(self, mock_as_string):
         mock_as_string.assert_called_once_with()
 
 
+class TestSitePackagesGroup(unittest.TestCase):
+    @mock.patch('importanize.groups.is_site_package')
+    def test_should_add_statement(self, mock_is_std_lib):
+        statement = mock.MagicMock()
+        actual = SitePackagesGroup().should_add_statement(statement)
+        self.assertEqual(actual, mock_is_std_lib.return_value)
+        mock_is_std_lib.assert_called_once_with(statement.root_module)
+
+
 class TestStdLibGroup(unittest.TestCase):
     @mock.patch('importanize.groups.is_std_lib')
     def test_should_add_statement(self, mock_is_std_lib):

tests/test_main.py

@@ -53,6 +53,8 @@ def test_run_importanize_print(self, mock_print):
                 mock.MagicMock(print=True,
                                formatter='grouped'))
         )
+
+        # self.assertMultiLineEqual(mock_print.call_args[0][0], expected)
         mock_print.assert_called_once_with(expected)
 
     @mock.patch(TESTING_MODULE + '.print', create=True)

tests/test_utils.py

@@ -9,24 +9,38 @@
 from importanize.utils import (
     ignore_site_packages_paths,
     is_std_lib,
+    is_site_package,
     list_strip,
     read,
 )
 
 
 class TestUtils(unittest.TestCase):
-    def test_ignore_site_packages_paths(self):
+    def _test_ignore_site_packages_paths(self, raise_msg=None):
         sys.path.append(os.getcwd())
         paths = sys.path[:]
 
-        with ignore_site_packages_paths():
-            self.assertNotEqual(sys.path, paths)
-            self.assertLess(len(sys.path), len(paths))
+        try:
+            with ignore_site_packages_paths():
+                self.assertNotEqual(sys.path, paths)
+                self.assertLess(len(sys.path), len(paths))
+                if raise_msg:
+                    raise ValueError(raise_msg)
+        except ValueError as e:
+            if raise_msg not in str(e):
+                # -- This only happens if there's a bug in this test
+                raise  # pragma: no cover
 
         self.assertIn(os.getcwd(), sys.path)
         self.assertListEqual(sys.path, paths)
         sys.path.remove(os.getcwd())
 
+    def test_site_packages_paths(self):
+        self._test_ignore_site_packages_paths(raise_msg=None)
+
+    def test_site_packages_paths_exception(self):
+        self._test_ignore_site_packages_paths(raise_msg="TEST EXCEPTION")
+
     def test_is_std_lib(self):
         self.assertFalse(is_std_lib(''))
 
@@ -88,6 +102,34 @@ def test_is_std_lib(self):
 
         self.assertFalse(is_std_lib('foo'))
 
+    def test_is_site_package(self):
+        self.assertFalse(is_site_package(''))
+
+        # -- Be sure that stdlib modules are not site-packages
+        stdlib_modules = (
+            'argparse',
+            'codecs',
+        )
+        for module in stdlib_modules:
+            msg = '{} should not be sitepackages'
+            self.assertFalse(is_site_package(module),
+                             msg.format(module))
+
+        # -- Be sure that fake modules are not site-packages
+        self.assertFalse(is_site_package('foo'))
+
+        # -- These packages come from requirements-dev.txt
+        site_packages_modules = (
+            "coverage",
+            "mock",
+            "rednose",
+            "tox",
+        )
+        for module in site_packages_modules:
+            msg = '{} should  be sitepackages'
+            self.assertTrue(is_site_package(module),
+                            msg.format(module))
+
     def test_list_strip(self):
         self.assertListEqual(
             list_strip(['  hello ', 'world']),