Repository: timothycrosley/isort (python)

Issue:
Monkey patching urllib not ideal
We recently had some (python 3 only) code that was erroneously doing
```
from urllib import quote
```
which was working in tests but not in production. The reason being that isort was being loaded in tests and monkey patches urllib at https://github.com/timothycrosley/isort/blob/develop/isort/pie_slice.py#L115. It would have been nice not to have to debug this, so can we update isort not to do this patch?

It doesn't even seem to use urllib...

Pull request description:
Remove all unused code from pie_slice.py
Some of the unused code previously monkey patched stdlib modules which
caused unexpected results in other projects.

Fixes #801

Patch:
diff --git a/isort/__main__.py b/isort/__main__.py
index 060f7331..186c98e8 100644
--- a/isort/__main__.py
+++ b/isort/__main__.py
@@ -1,8 +1,5 @@
 from __future__ import absolute_import
 
-from .pie_slice import apply_changes_to_python_environment
-apply_changes_to_python_environment()
-
-from isort.main import main  # noqa: E402
+from isort.main import main
 
 main()
diff --git a/isort/isort.py b/isort/isort.py
index 01878f96..5038ecec 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -39,7 +39,7 @@ from difflib import unified_diff
 from . import settings
 from .finders import FindersManager
 from .natural import nsorted
-from .pie_slice import input, itemsview
+from .pie_slice import input
 
 
 class SortImports(object):
@@ -53,7 +53,7 @@ class SortImports(object):
         settings_path = settings_path or os.getcwd()
 
         self.config = settings.from_path(settings_path).copy()
-        for key, value in itemsview(setting_overrides):
+        for key, value in setting_overrides.items():
             access_key = key.replace('not_', '').lower()
             # The sections config needs to retain order and can't be converted to a set.
             if access_key != 'sections' and type(self.config.get(access_key)) in (list, tuple):
@@ -85,7 +85,7 @@ class SortImports(object):
         self.import_placements = {}
         self.remove_imports = [self._format_simplified(removal) for removal in self.config['remove_imports']]
         self.add_imports = [self._format_natural(addition) for addition in self.config['add_imports']]
-        self._section_comments = ["# " + value for key, value in itemsview(self.config) if
+        self._section_comments = ["# " + value for key, value in self.config.items() if
                                   key.startswith('import_heading') and value]
 
         self.file_encoding = 'utf-8'
diff --git a/isort/main.py b/isort/main.py
index c7acddfc..9356c6a3 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -32,8 +32,6 @@ import setuptools
 from isort import SortImports, __version__
 from isort.settings import DEFAULT_SECTIONS, default, from_path, should_skip
 
-from .pie_slice import itemsview
-
 INTRO = r"""
 /#######################################################################\
 
@@ -129,14 +127,14 @@ class ISortCommand(setuptools.Command):
 
     def initialize_options(self):
         default_settings = default.copy()
-        for (key, value) in itemsview(default_settings):
+        for key, value in default_settings.items():
             setattr(self, key, value)
 
     def finalize_options(self):
         "Get options from config files."
         self.arguments = {}
         computed_settings = from_path(os.getcwd())
-        for (key, value) in itemsview(computed_settings):
+        for key, value in computed_settings.items():
             self.arguments[key] = value
 
     def distribution_files(self):
@@ -289,7 +287,7 @@ def create_parser():
                         help='Tells isort to apply changes recursively without asking')
     parser.add_argument('files', nargs='*', help='One or more Python source files that need their imports sorted.')
 
-    arguments = {key: value for key, value in itemsview(vars(parser.parse_args())) if value}
+    arguments = {key: value for key, value in vars(parser.parse_args()).items() if value}
     if 'dont_order_by_type' in arguments:
         arguments['order_by_type'] = False
     return arguments
diff --git a/isort/pie_slice.py b/isort/pie_slice.py
index b828e23b..2209cb4e 100644
--- a/isort/pie_slice.py
+++ b/isort/pie_slice.py
@@ -30,265 +30,13 @@ PY2 = sys.version_info[0] == 2
 PY3 = sys.version_info[0] == 3
 VERSION = sys.version_info
 
-native_dict = dict
-native_round = round
-native_filter = filter
-native_map = map
-native_zip = zip
-native_range = range
-native_str = str
-native_chr = chr
-native_input = input
-native_next = next
-native_object = object
-
-common = ['native_dict', 'native_round', 'native_filter', 'native_map', 'native_range', 'native_str', 'native_chr',
-          'native_input', 'PY2', 'PY3', 'u', 'itemsview', 'valuesview', 'keysview', 'execute', 'integer_types',
-          'native_next', 'native_object', 'with_metaclass', 'lru_cache']
-
-
-def with_metaclass(meta, *bases):
-    """Enables use of meta classes across Python Versions. taken from jinja2/_compat.py.
-
-    Use it like this::
-
-        class BaseForm(object):
-            pass
-
-        class FormType(type):
-            pass
-
-        class Form(with_metaclass(FormType, BaseForm)):
-            pass
-
-    """
-    class metaclass(meta):
-        __call__ = type.__call__
-        __init__ = type.__init__
-
-        def __new__(cls, name, this_bases, d):
-            if this_bases is None:
-                return type.__new__(cls, name, (), d)
-            return meta(name, bases, d)
-    return metaclass('temporary_class', None, {})
-
-
-def unmodified_isinstance(*bases):
-    """When called in the form
-
-    MyOverrideClass(unmodified_isinstance(BuiltInClass))
-
-    it allows calls against passed in built in instances to pass even if there not a subclass
-
-    """
-    class UnmodifiedIsInstance(type):
-        @classmethod
-        def __instancecheck__(cls, instance):
-            if cls.__name__ in (str(base.__name__) for base in bases):
-                return isinstance(instance, bases)
-
-            return type.__instancecheck__(cls, instance)
-
-    return with_metaclass(UnmodifiedIsInstance, *bases)
+__all__ = ['PY2', 'PY3', 'lru_cache']
 
 
 if PY3:
-    import urllib
-    import builtins
-    from urllib import parse
-
     input = input
-    integer_types = (int, )
-
-    def u(string):
-        return string
-
-    def itemsview(collection):
-        return collection.items()
-
-    def valuesview(collection):
-        return collection.values()
-
-    def keysview(collection):
-        return collection.keys()
-
-    urllib.quote = parse.quote
-    urllib.quote_plus = parse.quote_plus
-    urllib.unquote = parse.unquote
-    urllib.unquote_plus = parse.unquote_plus
-    urllib.urlencode = parse.urlencode
-    execute = getattr(builtins, 'exec')
-    if VERSION[1] < 2:
-        def callable(entity):
-            return hasattr(entity, '__call__')
-        common.append('callable')
-
-    def apply_changes_to_python_environment():
-        pass
-
-    __all__ = common + ['urllib', 'apply_changes_to_python_environment']
 else:
-    from itertools import ifilter as filter  # noqa: F401
-    from itertools import imap as map  # noqa: F401
-    from itertools import izip as zip  # noqa: F401
-    from decimal import Decimal, ROUND_HALF_EVEN
-
-    try:
-        str = unicode
-        chr = unichr
-        input = raw_input
-        range = xrange
-        integer_types = (int, long)
-    except NameError:  # Python 3
-        sys.exit('This should not happen!')
-
-    import sys
-    stdout = sys.stdout
-    stderr = sys.stderr
-
-    def _create_not_allowed(name):
-        def _not_allow(*args, **kwargs):
-            raise NameError("name '{0}' is not defined".format(name))
-        _not_allow.__name__ = name
-        return _not_allow
-
-    python_environment_changes_applied = False
-
-    def apply_changes_to_python_environment():
-        global python_environment_changes_applied
-        if python_environment_changes_applied:
-            return
-
-        try:
-            reload(sys)
-            sys.stdout = stdout
-            sys.stderr = stderr
-            sys.setdefaultencoding('utf-8')
-        except NameError:  # Python 3
-            sys.exit('This should not happen!')
-
-        for removed in ('apply', 'cmp', 'coerce', 'execfile', 'raw_input', 'unpacks'):
-            globals()[removed] = _create_not_allowed(removed)
-
-        python_environment_changes_applied = True
-
-    def u(s):
-        try:
-            if isinstance(s, unicode):
-                return s
-            else:
-                return unicode(s.replace(r'\\', r'\\\\'), "unicode_escape")
-        except NameError:  # Python 3
-            return str(s)
-
-    def execute(_code_, _globs_=None, _locs_=None):
-        """Execute code in a namespace."""
-        if _globs_ is None:
-            frame = sys._getframe(1)
-            _globs_ = frame.f_globals
-            if _locs_ is None:
-                _locs_ = frame.f_locals
-            del frame
-        elif _locs_ is None:
-            _locs_ = _globs_
-        exec("""exec _code_ in _globs_, _locs_""")
-
-    class _dict_view_base(object):
-        __slots__ = ('_dictionary', )
-
-        def __init__(self, dictionary):
-            self._dictionary = dictionary
-
-        def __repr__(self):
-            return "{0}({1})".format(self.__class__.__name__, str(list(self.__iter__())))
-
-        def __unicode__(self):
-            return str(self.__repr__())
-
-        def __str__(self):
-            return str(self.__unicode__())
-
-    class dict_keys(_dict_view_base):
-        __slots__ = ()
-
-        def __iter__(self):
-            return self._dictionary.iterkeys()
-
-    class dict_values(_dict_view_base):
-        __slots__ = ()
-
-        def __iter__(self):
-            return self._dictionary.itervalues()
-
-    class dict_items(_dict_view_base):
-        __slots__ = ()
-
-        def __iter__(self):
-            return self._dictionary.iteritems()
-
-    def itemsview(collection):
-        return dict_items(collection)
-
-    def valuesview(collection):
-        return dict_values(collection)
-
-    def keysview(collection):
-        return dict_keys(collection)
-
-    class dict(unmodified_isinstance(native_dict)):
-        def has_key(self, *args, **kwargs):
-            return AttributeError("'dict' object has no attribute 'has_key'")
-
-        def items(self):
-            return dict_items(self)
-
-        def keys(self):
-            return dict_keys(self)
-
-        def values(self):
-            return dict_values(self)
-
-    def round(number, ndigits=None):
-        return_int = False
-        if ndigits is None:
-            return_int = True
-            ndigits = 0
-        if hasattr(number, '__round__'):
-            return number.__round__(ndigits)
-
-        if ndigits < 0:
-            raise NotImplementedError('negative ndigits not supported yet')
-        exponent = Decimal('10') ** (-ndigits)
-        d = Decimal.from_float(number).quantize(exponent,
-                                                rounding=ROUND_HALF_EVEN)
-        if return_int:
-            return int(d)
-        else:
-            return float(d)
-
-    def next(iterator):
-        try:
-            iterator.__next__()
-        except Exception:
-            native_next(iterator)
-
-    class FixStr(type):
-        def __new__(cls, name, bases, dct):
-            if '__str__' in dct:
-                dct['__unicode__'] = dct['__str__']
-            dct['__str__'] = lambda self: self.__unicode__().encode('utf-8')
-            return type.__new__(cls, name, bases, dct)
-
-        def __instancecheck__(cls, instance):
-            if cls.__name__ == "object":
-                return isinstance(instance, native_object)
-            return type.__instancecheck__(cls, instance)
-
-    class object(with_metaclass(FixStr, object)):
-        pass
-
-    __all__ = common + ['round', 'dict', 'apply', 'cmp', 'coerce', 'execfile', 'raw_input', 'unpacks', 'str', 'chr',
-                        'input', 'range', 'filter', 'map', 'zip', 'object']
+    input = raw_input  # noqa: F821
 
 
 if sys.version_info < (3, 2):
@@ -378,4 +126,4 @@ if sys.version_info < (3, 2):
         return decorating_function
 
 else:
-    from functools import lru_cache  # noqa: F401
+    from functools import lru_cache
diff --git a/isort/settings.py b/isort/settings.py
index 2c86842b..2d824263 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -33,7 +33,7 @@ import warnings
 from collections import namedtuple
 from distutils.util import strtobool
 
-from .pie_slice import itemsview, lru_cache, native_str
+from .pie_slice import lru_cache
 
 try:
     import configparser
@@ -164,7 +164,7 @@ def _update_settings_with_config(path, name, default, sections, computed_setting
     tries = 0
     current_directory = path
     while current_directory and tries < MAX_CONFIG_SEARCH_DEPTH:
-        potential_path = os.path.join(current_directory, native_str(name))
+        potential_path = os.path.join(current_directory, str(name))
         if os.path.exists(potential_path):
             editor_config_file = potential_path
             break
@@ -200,7 +200,7 @@ def _update_with_config_file(file_path, sections, computed_settings):
         if max_line_length:
             computed_settings['line_length'] = float('inf') if max_line_length == 'off' else int(max_line_length)
 
-    for key, value in itemsview(settings):
+    for key, value in settings.items():
         access_key = key.replace('not_', '').lower()
         existing_value_type = type(default.get(access_key, ''))
         if existing_value_type in (list, tuple):


Test patch:
diff --git a/test_isort.py b/test_isort.py
index d7d770af..25ad87e7 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -2584,3 +2584,11 @@ def test_pipfile_finder(tmpdir):
     assert finder._normalize_name('Flask-RESTful') == 'flask_restful'  # conver `-`to `_`
 
     pipfile.remove()
+
+
+@pytest.mark.skipif(sys.version_info[0] == 2, reason="Requires Python 3")
+def test_monkey_patched_urllib():
+    with pytest.raises(ImportError):
+        # Previous versions of isort monkey patched urllib which caused unusual
+        # importing for other projects.
+        from urllib import quote  # noqa: F401