Issue #14582: Import returns the module returned by a loader instead

of sys.modules when possible.

This is being done for two reasons. One is to gain a little bit of
performance by skipping an unnecessary dict lookup in sys.modules. But
the other (and main) reason is to be a little bit more clear in how
things should work from the perspective of import's interactions with
loaders. Otherwise loaders can easily forget to return the module even
though PEP 302 explicitly states they are expected to return the module
they loaded.

Author: brettcannon

Lib/importlib/_bootstrap.py

@@ -974,12 +974,12 @@ def _find_and_load(name, import_):
     loader = _find_module(name, path)
     if loader is None:
         raise ImportError(_ERR_MSG.format(name), name=name)
-    elif name not in sys.modules:
-        # The parent import may have already imported this module.
-        loader.load_module(name)
+    elif name in sys.modules:
+        # The parent module already imported this module.
+        module = sys.modules[name]
+    else:
+        module = loader.load_module(name)
         verbose_message('import {!r} # {!r}', name, loader)
-    # Backwards-compatibility; be nicer to skip the dict lookup.
-    module = sys.modules[name]
     if parent:
         # Set the module as an attribute on its parent.
         parent_module = sys.modules[parent]
@@ -1078,7 +1078,11 @@ def __import__(name, globals={}, locals={}, fromlist=[], level=0):
         # Return up to the first dot in 'name'. This is complicated by the fact
         # that 'name' may be relative.
         if level == 0:
-            return sys.modules[name.partition('.')[0]]
+            index = name.find('.')
+            if index == -1:
+                return module
+            else:
+                return sys.modules[name[:index]]
         elif not name:
             return module
         else:

Lib/importlib/test/import_/test_caching.py

@@ -47,36 +47,12 @@ def load_module(self, fullname):
         mock.load_module = MethodType(load_module, mock)
         return mock
 
-    # __import__ inconsistent between loaders and built-in import when it comes
-    #   to when to use the module in sys.modules and when not to.
-    @import_util.importlib_only
-    def test_using_cache_after_loader(self):
-        # [from cache on return]
-        with self.create_mock('module') as mock:
+    def test_using_loader_return(self):
+        loader_return = 'hi there!'
+        with self.create_mock('module', return_=loader_return) as mock:
             with util.import_state(meta_path=[mock]):
                 module = import_util.import_('module')
-                self.assertEqual(id(module), id(sys.modules['module']))
-
-    # See test_using_cache_after_loader() for reasoning.
-    @import_util.importlib_only
-    def test_using_cache_for_assigning_to_attribute(self):
-        # [from cache to attribute]
-        with self.create_mock('pkg.__init__', 'pkg.module') as importer:
-            with util.import_state(meta_path=[importer]):
-                module = import_util.import_('pkg.module')
-                self.assertTrue(hasattr(module, 'module'))
-                self.assertTrue(id(module.module), id(sys.modules['pkg.module']))
-
-    # See test_using_cache_after_loader() for reasoning.
-    @import_util.importlib_only
-    def test_using_cache_for_fromlist(self):
-        # [from cache for fromlist]
-        with self.create_mock('pkg.__init__', 'pkg.module') as importer:
-            with util.import_state(meta_path=[importer]):
-                module = import_util.import_('pkg', fromlist=['module'])
-                self.assertTrue(hasattr(module, 'module'))
-                self.assertEqual(id(module.module),
-                                 id(sys.modules['pkg.module']))
+                self.assertEqual(module, loader_return)
 
 
 def test_main():

Misc/NEWS

@@ -10,6 +10,9 @@ What's New in Python 3.3.0 Alpha 3?
 Core and Builtins
 -----------------
 
+- Issue #14582: Import directly returns the module as returned by a loader when
+  possible instead of fetching it from sys.modules.
+
 - Issue #13889: Check and (if necessary) set FPU control word before calling
   any of the dtoa.c string <-> float conversion functions, on MSVC builds of
   Python.  This fixes issues when embedding Python in a Delphi app.

Python/import.c

@@ -3019,15 +3019,22 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals,
             Py_DECREF(partition);
 
             if (level == 0) {
-                final_mod = PyDict_GetItem(interp->modules, front);
-                Py_DECREF(front);
-                if (final_mod == NULL) {
-                    PyErr_Format(PyExc_KeyError,
-                                 "%R not in sys.modules as expected", front);
+                if (PyUnicode_GET_LENGTH(name) ==
+                        PyUnicode_GET_LENGTH(front)) {
+                    final_mod = mod;
                 }
                 else {
-                    Py_INCREF(final_mod);
+                    final_mod = PyDict_GetItem(interp->modules, front);
+                    if (final_mod == NULL) {
+                        PyErr_Format(PyExc_KeyError,
+                                     "%R not in sys.modules as expected", front);
+                    }
+                }
+                Py_DECREF(front);
+                if (final_mod == NULL) {
+                    goto error_with_unlock;
                 }
+                Py_INCREF(final_mod);
             }
             else {
                 Py_ssize_t cut_off = PyUnicode_GetLength(name) -