bpo-26131: deprecate the use of load_module()

python · brettcannon · Dec 4, 2020 · Oct 23, 2020 · Oct 23, 2020 · Oct 27, 2020
commit 4c7284cf6b6952ebf3941a1bf3be6128a4b8ad77
diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst
@@ -262,11 +262,23 @@ Optimizations
   with ``gcc`` by up to 30%. See `this article
   <https://developers.redhat.com/blog/2020/06/25/red-hat-enterprise-linux-8-2-brings-faster-python-3-8-run-speeds/>`_
   for more details. (Contributed by Victor Stinner and Pablo Galindo in
-  :issue:`38980`)
+  :issue:`38980`.)
+
 
 Deprecated
 ==========
 
+* The various ``load_module()`` methods of :mod:`importlib` have been
+  documented as deprecated since Python 3.6, but will now also trigger
+  a :exc:`DeprecationWarning`. Use
+  :meth:`~importlib.abc.Loader.exec_module` instead.
+  (Contributed by Brett Cannon in :issue:`26131`.)
+
+* The use of :meth:`~importlib.abc.Loader.load_module` by the import
+  system now triggers an :exc:`ImportWarning` as
+  :meth:`~importlib.abc.Loader.exec_module` is preferred.
+  (Contributed by Brett Cannon in :issue:`26131`.)
+
 
 Removed
 =======

diff --git a/Lib/importlib/_abc.py b/Lib/importlib/_abc.py
@@ -35,6 +35,7 @@ def load_module(self, fullname):
         """
         if not hasattr(self, 'exec_module'):
             raise ImportError
+        # Warning implemented in _load_module_shim().
         return _bootstrap._load_module_shim(self, fullname)
 
     def module_repr(self, module):

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
@@ -265,6 +265,9 @@ def _load_module_shim(self, fullname):
     This method is deprecated.  Use loader.exec_module instead.
 
     """
+    msg = ("the load_module() method is deprecated and slated for removal in "
+          "Python 3.12; use exec_module() instead")
+    _warnings.warn(msg, DeprecationWarning)
     spec = spec_from_loader(fullname, self)
     if fullname in sys.modules:
         module = sys.modules[fullname]
@@ -605,9 +608,9 @@ def _exec(spec, module):
             else:
                 _init_module_attrs(spec, module, override=True)
                 if not hasattr(spec.loader, 'exec_module'):
-                    # (issue19713) Once BuiltinImporter and ExtensionFileLoader
-                    # have exec_module() implemented, we can add a deprecation
-                    # warning here.
+                    msg = ("loader.exec_module() not found; "
+                           "falling back to load_module()")
+                    _warnings.warn(msg, ImportWarning)
                     spec.loader.load_module(name)
                 else:
                     spec.loader.exec_module(module)
@@ -620,9 +623,8 @@ def _exec(spec, module):
 
 
 def _load_backward_compatible(spec):
-    # (issue19713) Once BuiltinImporter and ExtensionFileLoader
-    # have exec_module() implemented, we can add a deprecation
-    # warning here.
+    # It is assumed that all callers have warned about using load_module()
+    # appropriately.
     try:
         spec.loader.load_module(spec.name)
     except:
@@ -661,6 +663,8 @@ def _load_unlocked(spec):
     if spec.loader is not None:
         # Not a namespace package.
         if not hasattr(spec.loader, 'exec_module'):
+            msg = "loader.exec_module() not found; falling back to load_module()"
+            _warnings.warn(msg, ImportWarning)
             return _load_backward_compatible(spec)
 
     module = module_from_spec(spec)
@@ -844,6 +848,7 @@ def load_module(cls, fullname):
         This method is deprecated.  Use exec_module() instead.
 
         """
+        # Warning about deprecation implemented in _load_module_shim().
         return _load_module_shim(cls, fullname)
 
     @classmethod

diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
@@ -791,7 +791,8 @@ def exec_module(self, module):
         _bootstrap._call_with_frames_removed(exec, code, module.__dict__)
 
     def load_module(self, fullname):
-        """This module is deprecated."""
+        """This method is deprecated."""
+        # Warning implemented in _load_module_shim().
         return _bootstrap._load_module_shim(self, fullname)
 
 
@@ -966,7 +967,7 @@ def load_module(self, fullname):
         """
         # The only reason for this method is for the name check.
         # Issue #14857: Avoid the zero-argument form of super so the implementation
-        # of that form can be updated without breaking the frozen module
+        # of that form can be updated without breaking the frozen module.
         return super(FileLoader, self).load_module(fullname)
 
     @_check_name
@@ -1216,6 +1217,7 @@ def load_module(self, fullname):
         # The import system never calls this method.
         _bootstrap._verbose_message('namespace module loaded with path {!r}',
                                     self._path)
+        # Warning implemented in _load_module_shim().
         return _bootstrap._load_module_shim(self, fullname)
 
 

diff --git a/Lib/test/test_importlib/builtin/test_loader.py b/Lib/test/test_importlib/builtin/test_loader.py
@@ -6,6 +6,7 @@
 import sys
 import types
 import unittest
+import warnings
 
 @unittest.skipIf(util.BUILTINS.good_name is None, 'no reasonable builtin module')
 class LoaderTests(abc.LoaderTests):
@@ -24,7 +25,9 @@ def verify(self, module):
         self.assertIn(module.__name__, sys.modules)
 
     def load_module(self, name):
-        return self.machinery.BuiltinImporter.load_module(name)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            return self.machinery.BuiltinImporter.load_module(name)
 
     def test_module(self):
         # Common case.

diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py
@@ -1,3 +1,4 @@
+from warnings import catch_warnings
 from .. import abc
 from .. import util
 
@@ -7,6 +8,7 @@
 import sys
 import types
 import unittest
+import warnings
 import importlib.util
 import importlib
 from test.support.script_helper import assert_python_failure
@@ -20,14 +22,18 @@ def setUp(self):
                                                          util.EXTENSIONS.file_path)
 
     def load_module(self, fullname):
-        return self.loader.load_module(fullname)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            return self.loader.load_module(fullname)
 
     def test_load_module_API(self):
         # Test the default argument for load_module().
-        self.loader.load_module()
-        self.loader.load_module(None)
-        with self.assertRaises(ImportError):
-            self.load_module('XXX')
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            self.loader.load_module()
+            self.loader.load_module(None)
+            with self.assertRaises(ImportError):
+                self.load_module('XXX')
 
     def test_equality(self):
         other = self.machinery.ExtensionFileLoader(util.EXTENSIONS.name,
@@ -94,6 +100,21 @@ def setUp(self):
         self.loader = self.machinery.ExtensionFileLoader(
             self.name, self.spec.origin)
 
+    def load_module(self):
+        '''Load the module from the test extension'''
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            return self.loader.load_module(self.name)
+
+    def load_module_by_name(self, fullname):
+        '''Load a module from the test extension by name'''
+        origin = self.spec.origin
+        loader = self.machinery.ExtensionFileLoader(fullname, origin)
+        spec = importlib.util.spec_from_loader(fullname, loader)
+        module = importlib.util.module_from_spec(spec)
+        loader.exec_module(module)
+        return module
+
     # No extension module as __init__ available for testing.
     test_package = None
 
@@ -157,19 +178,6 @@ def test_try_registration(self):
             with self.assertRaises(SystemError):
                 module.call_state_registration_func(2)
 
-    def load_module(self):
-        '''Load the module from the test extension'''
-        return self.loader.load_module(self.name)
-
-    def load_module_by_name(self, fullname):
-        '''Load a module from the test extension by name'''
-        origin = self.spec.origin
-        loader = self.machinery.ExtensionFileLoader(fullname, origin)
-        spec = importlib.util.spec_from_loader(fullname, loader)
-        module = importlib.util.module_from_spec(spec)
-        loader.exec_module(module)
-        return module
-
     def test_load_submodule(self):
         '''Test loading a simulated submodule'''
         module = self.load_module_by_name('pkg.' + self.name)

diff --git a/Lib/test/test_importlib/frozen/test_loader.py b/Lib/test/test_importlib/frozen/test_loader.py
@@ -161,19 +161,23 @@ def test_module_repr(self):
                              "<module '__hello__' (frozen)>")
 
     def test_module_repr_indirect(self):
-        with util.uncache('__hello__'), captured_stdout():
-            module = self.machinery.FrozenImporter.load_module('__hello__')
-        self.assertEqual(repr(module),
-                         "<module '__hello__' (frozen)>")
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            with util.uncache('__hello__'), captured_stdout():
+                module = self.machinery.FrozenImporter.load_module('__hello__')
+            self.assertEqual(repr(module),
+                            "<module '__hello__' (frozen)>")
 
     # No way to trigger an error in a frozen module.
     test_state_after_failure = None
 
     def test_unloadable(self):
-        assert self.machinery.FrozenImporter.find_module('_not_real') is None
-        with self.assertRaises(ImportError) as cm:
-            self.machinery.FrozenImporter.load_module('_not_real')
-        self.assertEqual(cm.exception.name, '_not_real')
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            assert self.machinery.FrozenImporter.find_module('_not_real') is None
+            with self.assertRaises(ImportError) as cm:
+                self.machinery.FrozenImporter.load_module('_not_real')
+            self.assertEqual(cm.exception.name, '_not_real')
 
 
 (Frozen_LoaderTests,

diff --git a/Lib/test/test_importlib/import_/test___loader__.py b/Lib/test/test_importlib/import_/test___loader__.py
@@ -2,6 +2,7 @@
 import sys
 import types
 import unittest
+import warnings
 
 from .. import util
 
@@ -45,25 +46,29 @@ def load_module(self, fullname):
 class LoaderAttributeTests:
 
     def test___loader___missing(self):
-        module = types.ModuleType('blah')
-        try:
-            del module.__loader__
-        except AttributeError:
-            pass
-        loader = LoaderMock()
-        loader.module = module
-        with util.uncache('blah'), util.import_state(meta_path=[loader]):
-            module = self.__import__('blah')
-        self.assertEqual(loader, module.__loader__)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            module = types.ModuleType('blah')
+            try:
+                del module.__loader__
+            except AttributeError:
+                pass
+            loader = LoaderMock()
+            loader.module = module
+            with util.uncache('blah'), util.import_state(meta_path=[loader]):
+                module = self.__import__('blah')
+            self.assertEqual(loader, module.__loader__)
 
     def test___loader___is_None(self):
-        module = types.ModuleType('blah')
-        module.__loader__ = None
-        loader = LoaderMock()
-        loader.module = module
-        with util.uncache('blah'), util.import_state(meta_path=[loader]):
-            returned_module = self.__import__('blah')
-        self.assertEqual(loader, module.__loader__)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            module = types.ModuleType('blah')
+            module.__loader__ = None
+            loader = LoaderMock()
+            loader.module = module
+            with util.uncache('blah'), util.import_state(meta_path=[loader]):
+                returned_module = self.__import__('blah')
+            self.assertEqual(loader, module.__loader__)
 
 
 (Frozen_Tests,

diff --git a/Lib/test/test_importlib/import_/test___package__.py b/Lib/test/test_importlib/import_/test___package__.py
@@ -98,6 +98,16 @@ def __init__(self, parent):
 class Using__package__PEP302(Using__package__):
     mock_modules = util.mock_modules
 
+    def test_using___package__(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_using___package__()
+
+    def test_spec_fallback(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_spec_fallback()
+
 
 (Frozen_UsingPackagePEP302,
  Source_UsingPackagePEP302
@@ -155,6 +165,21 @@ def test_submodule(self):
 class Setting__package__PEP302(Setting__package__, unittest.TestCase):
     mock_modules = util.mock_modules
 
+    def test_top_level(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_top_level()
+
+    def test_package(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_package()
+
+    def test_submodule(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_submodule()
+
 class Setting__package__PEP451(Setting__package__, unittest.TestCase):
     mock_modules = util.mock_spec
 

diff --git a/Lib/test/test_importlib/import_/test_api.py b/Lib/test/test_importlib/import_/test_api.py
@@ -4,6 +4,7 @@
 import sys
 import types
 import unittest
+import warnings
 
 PKG_NAME = 'fine'
 SUBMOD_NAME = 'fine.bogus'
@@ -100,6 +101,36 @@ def test_blocked_fromlist(self):
 class OldAPITests(APITest):
     bad_finder_loader = BadLoaderFinder
 
+    def test_raises_ModuleNotFoundError(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_raises_ModuleNotFoundError()
+
+    def test_name_requires_rparition(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_name_requires_rparition()
+
+    def test_negative_level(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_negative_level()
+
+    def test_nonexistent_fromlist_entry(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_nonexistent_fromlist_entry()
+
+    def test_fromlist_load_error_propagates(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_fromlist_load_error_propagates
+
+    def test_blocked_fromlist(self):
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", ImportWarning)
+            super().test_blocked_fromlist()
+
 
 (Frozen_OldAPITests,
  Source_OldAPITests