Issue 37521: importlib examples have their exec_module()/sys.modules assignment lines reversed

unzip the attached zip file and run main.py

expected output:

True
<module 'testext.sub' (namespace)>

actual output:
False
<module 'testext.sub' (namespace)>

So what?
If you follow these directions, https://docs.python.org/3.7/library/importlib.html#checking-if-a-module-can-be-imported , you will put a stale reference in sys.modules even though a fresh reference is already there. If you use that code on a package with a subpackage, the subpackage will not be set as an attribute on the package.

Updated main.py.

New expected output:
====================
True
<module 'testext.sub' (namespace)>
<module 'testext.sub' (namespace)>

New actual output:
==================
False
<module 'testext.sub' (namespace)>
Traceback (most recent call last):
  File "main.py", line 14, in <module>
    print(module.sub)
AttributeError: module 'testext' has no attribute 'sub'

Mind re-uploading the examples files with a more appropriate file name? (Saying "import is f-'ed" is not motivating to those of us who wrote it to try and fix this.)

Updated. Also removed some old code which had irrelevant imports that I thought I removed already. The expected and actual output remain the same.

Thanks! I'll try to have a look when I can.

So the issue is the lines assigning to sys.modules and loader.exec_module() are reversed. Had you not done an import in your testext.__init__ you never would have noticed, but since you are then the normal import system is noticing there's nothing in sys.modules and thus inserting a module (importlib.uilt.module_from_spec() always creates a new module so your module that you create never ends up in sys.modules).

I will have a PR up shortly to fix the docs.

New changeset 0827064c955f88df8ba5621d8e3d81be3cfc26a9 by Miss Islington (bot) (Brett Cannon) in branch 'master':
bpo-37521: No longer treat insertion into sys.modules as optional in importlib examples (GH-14723)
https://github.com/python/cpython/commit/0827064c955f88df8ba5621d8e3d81be3cfc26a9

New changeset bfb709b771230e7e466961b5ddf4852962602450 by Miss Islington (bot) in branch '3.8':
[3.8] bpo-37521: No longer treat insertion into sys.modules as optional in importlib examples (GH-14723) (GH-14724)
https://github.com/python/cpython/commit/bfb709b771230e7e466961b5ddf4852962602450

Thanks for the report, Benjamin!

Hmm, why is the assignment to sys.modules necessary at all, if module_from_spec() always does so?

I'm not sure why you think importlib.util.module_from_spec() adds a module to sys.modules? https://docs.python.org/3/library/importlib.html#importlib.util.module_from_spec doesn't say that nor does https://github.com/python/cpython/blob/master/Lib/importlib/_bootstrap.py#L549 actually do that.

I dunno, one of those three does. I suppose it's spec.loader.exec_module().

Nope, loader.exec_module() doesn't either: https://docs.python.org/3/library/importlib.html#importlib.abc.Loader.exec_module, https://github.com/python/cpython/blob/master/Lib/importlib/_bootstrap_external.py#L776.

So unfortunately I'm still not sure what you're talking about.

```
name = 'testext'

spec = importlib.util.find_spec(name)
print(name in sys.modules)
module = importlib.util.module_from_spec(spec)
print(name in sys.modules)
spec.loader.exec_module(module)
print(name in sys.modules)
```

This prints False, False, True though.

If you're using your example code for 'testext' that you uploaded then that's expected because the import in your package causes a normal import which means the import machinery is adding things to sys.modules. Try it with a plain module that's empty so you don't have any other imports as a side-effect of imports and you will see the module isn't added to sys.modules.

stage: patch review -> needs patch

messages: + msg347582

messages: + msg347483