Stop using the deprecated imp module when possible

CHANGES.md

@@ -4,8 +4,14 @@ master
 - Ensure that unpickling a function defined in a dynamic module several times
   sequentially does not reset the values of global variables.
   ([issue #187](https://github.com/cloudpipe/cloudpickle/issues/205))
+
 - Restrict the ability to pickle annotations to python3.7+ ([issue #193](
-  https://github.com/cloudpipe/cloudpickle/issues/193) and [issue #196](https://github.com/cloudpipe/cloudpickle/issues/196))
+  https://github.com/cloudpipe/cloudpickle/issues/193) and [issue #196](
+  https://github.com/cloudpipe/cloudpickle/issues/196))
+
+- Stop using the deprecated `imp` module when a solution based on
+  `importlib` is available.
+  ([issue #207](https://github.com/cloudpipe/cloudpickle/issues/207))
 
 
 0.5.6

cloudpickle/cloudpickle.py

@@ -44,7 +44,7 @@
 
 import dis
 from functools import partial
-import imp
+import importlib
 import io
 import itertools
 import logging
@@ -304,20 +304,10 @@ def save_module(self, obj):
         """
         Save a module as an import
         """
-        mod_name = obj.__name__
-        # If module is successfully found then it is not a dynamically created module
-        if hasattr(obj, '__file__'):
-            is_dynamic = False
-        else:
-            try:
-                _find_module(mod_name)
-                is_dynamic = False
-            except ImportError:
-                is_dynamic = True
-
         self.modules.add(obj)
-        if is_dynamic:
-            self.save_reduce(dynamic_subimport, (obj.__name__, vars(obj)), obj=obj)
+        if _is_dynamic(obj):
+            self.save_reduce(dynamic_subimport, (obj.__name__, vars(obj)),
+                             obj=obj)
         else:
             self.save_reduce(subimport, (obj.__name__,), obj=obj)
 
@@ -949,7 +939,7 @@ def subimport(name):
 
 
 def dynamic_subimport(name, vars):
-    mod = imp.new_module(name)
+    mod = types.ModuleType(name)
     mod.__dict__.update(vars)
     return mod
 
@@ -1146,19 +1136,31 @@ def _rehydrate_skeleton_class(skeleton_class, class_dict):
     return skeleton_class
 
 
-def _find_module(mod_name):
+def _is_dynamic(module):
     """
-    Iterate over each part instead of calling imp.find_module directly.
-    This function is able to find submodules (e.g. scikit.tree)
+    Return True if the module is special module that cannot be imported by its
+    name.
     """
-    path = None
-    for part in mod_name.split('.'):
-        if path is not None:
-            path = [path]
-        file, path, description = imp.find_module(part, path)
-        if file is not None:
-            file.close()
-    return path, description
+    # Quick check: module that have __file__ attribute are not dynamic modules.
+    if hasattr(module, '__file__'):
+        return False
+
+    if hasattr(module, '__spec__'):
+        return module.__spec__ is None
+    else:
+        # Backward compat for Python 2
+        import imp
+        try:
+            path = None
+            for part in module.__name__.split('.'):
+                if path is not None:
+                    path = [path]
+                f, path, description = imp.find_module(part, path)
+                if f is not None:
+                    f.close()
+        except ImportError:
+            return True
+        return False
 
 
 """Constructors for 3rd party libraries

tests/cloudpickle_test.py

@@ -4,8 +4,6 @@
 import collections
 import base64
 import functools
-import gc
-import imp
 from io import BytesIO
 import itertools
 import logging
@@ -17,6 +15,7 @@
 import subprocess
 import sys
 import textwrap
+import types
 import unittest
 import weakref
 import os
@@ -44,8 +43,8 @@
     tornado = None
 
 import cloudpickle
-from cloudpickle.cloudpickle import _find_module, _make_empty_cell, cell_set
-from cloudpickle.cloudpickle import _dynamic_modules_globals
+from cloudpickle.cloudpickle import _is_dynamic
+from cloudpickle.cloudpickle import _make_empty_cell, cell_set
 
 from .testutils import subprocess_pickle_echo
 from .testutils import assert_run_python_script
@@ -416,7 +415,7 @@ def test_module(self):
         self.assertEqual(pickle, pickle_clone)
 
     def test_dynamic_module(self):
-        mod = imp.new_module('mod')
+        mod = types.ModuleType('mod')
         code = '''
         x = 1
         def f(y):
@@ -450,7 +449,7 @@ def test_dynamic_modules_globals(self):
         # this dict in the child process, it will be garbage collected.
 
         # We first create a module
-        mod = imp.new_module('mod')
+        mod = types.ModuleType('mod')
         code = '''
         x = 1
         def func():
@@ -495,14 +494,13 @@ def func():
         finally:
             os.unlink(pickled_module_path)
 
-
     def test_load_dynamic_module_in_grandchild_process(self):
         # Make sure that when loaded, a dynamic module preserves its dynamic
         # property. Otherwise, this will lead to an ImportError if pickled in
         # the child process and reloaded in another one.
 
         # We create a new dynamic module
-        mod = imp.new_module('mod')
+        mod = types.ModuleType('mod')
         code = '''
         x = 1
         '''
@@ -588,16 +586,17 @@ def my_small_function(x, y):
         assert b'unwanted_function' not in b
         assert b'math' not in b
 
-    def test_find_module(self):
-        import pickle  # ensure this test is decoupled from global imports
-        _find_module('pickle')
+    def test_is_dynamic_module(self):
+        import pickle  # decouple this test from global imports
+        import os.path
 
-        with pytest.raises(ImportError):
-            _find_module('invalid_module')
+        assert not _is_dynamic(pickle)
+        assert not _is_dynamic(os.path)
 
-        with pytest.raises(ImportError):
-            valid_module = imp.new_module('valid_module')
-            _find_module('valid_module')
+        # user-created module without using the import machinery are also
+        # dynamic
+        dynamic_module = types.ModuleType('dynamic_module')
+        assert _is_dynamic(dynamic_module)
 
     def test_Ellipsis(self):
         self.assertEqual(Ellipsis,
@@ -614,7 +613,7 @@ def test_builtin_function_without_module(self):
 
         fi = itertools.chain.from_iterable
         fi_depickled = pickle_depickle(fi, protocol=self.protocol)
-        self.assertEqual(list(fi([[1, 2], [3, 4]])), [1, 2, 3, 4])
+        self.assertEqual(list(fi_depickled([[1, 2], [3, 4]])), [1, 2, 3, 4])
 
     @pytest.mark.skipif(tornado is None,
                         reason="test needs Tornado installed")
@@ -822,8 +821,7 @@ def foo(self):
         self.assertEqual(depickled_class().foo(), 'it works!')
         self.assertEqual(depickled_instance.foo(), 'it works!')
 
-        # assertRaises doesn't return a contextmanager in python 2.6 :(.
-        self.failUnlessRaises(TypeError, depickled_base)
+        self.assertRaises(TypeError, depickled_base)
 
         class DepickledBaseSubclass(depickled_base):
             def foo(self):
@@ -1079,7 +1077,7 @@ def test_function_from_dynamic_module_with_globals_modifications(self):
         # subsequent uplickling.
 
         # first, we create a dynamic module in the parent process
-        mod = imp.new_module('mod')
+        mod = types.ModuleType('mod')
         code = '''
         GLOBAL_STATE = "initial value"
 
@@ -1165,7 +1163,7 @@ def test_function_pickle_compat_0_4_0(self):
             b'\x01K\x01K\x01KCU\x04|\x00\x00Sq\x06N\x85q\x07)U\x01xq\x08\x85q'
             b'\tU\x07<stdin>q\nU\x08<lambda>q\x0bK\x01U\x00q\x0c))tq\rRq\x0eJ'
             b'\xff\xff\xff\xff}q\x0f\x87q\x10Rq\x11}q\x12N}q\x13NtR.')
-        self.assertEquals(42, cloudpickle.loads(pickled)(42))
+        self.assertEqual(42, cloudpickle.loads(pickled)(42))
 
     @pytest.mark.skipif(sys.version_info >= (3, 0),
                         reason="hardcoded pickle bytes for 2.7")
@@ -1179,7 +1177,7 @@ def test_function_pickle_compat_0_4_1(self):
             b'\tU\x07<stdin>q\nU\x08<lambda>q\x0bK\x01U\x00q\x0c))tq\rRq\x0eJ'
             b'\xff\xff\xff\xff}q\x0f\x87q\x10Rq\x11}q\x12N}q\x13U\x08__main__q'
             b'\x14NtR.')
-        self.assertEquals(42, cloudpickle.loads(pickled)(42))
+        self.assertEqual(42, cloudpickle.loads(pickled)(42))
 
     def test_pickle_reraise(self):
         for exc_type in [Exception, ValueError, TypeError, RuntimeError]:
@@ -1190,8 +1188,8 @@ def test_pickle_reraise(self):
     def test_unhashable_function(self):
         d = {'a': 1}
         depickled_method = pickle_depickle(d.get)
-        self.assertEquals(depickled_method('a'), 1)
-        self.assertEquals(depickled_method('b'), None)
+        self.assertEqual(depickled_method('a'), 1)
+        self.assertEqual(depickled_method('b'), None)
 
     def test_itertools_count(self):
         counter = itertools.count(1, step=2)