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

[iomintz]

BPO	37521
Nosy	@brettcannon, @ncoghlan, @ericsnowcurrently, @miss-islington, @iomintz
PRs	bpo-37521: No longer treat insertion into sys.modules as optional in importlib examples #14723 [3.8] bpo-37521: No longer treat insertion into sys.modules as optional in importlib examples (GH-14723) #14724
Files	importlib-util-module-from-spec-stale-reference.zip

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/brettcannon'
closed_at = <Date 2019-07-12.22:52:23.134>
created_at = <Date 2019-07-08.00:52:26.788>
labels = ['3.7', 'docs']
title = 'importlib examples have their exec_module()/sys.modules assignment lines reversed'
updated_at = <Date 2019-07-17.22:20:59.267>
user = 'https://github.com/iomintz'

bugs.python.org fields:

activity = <Date 2019-07-17.22:20:59.267>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2019-07-12.22:52:23.134>
closer = 'brett.cannon'
components = ['Documentation']
creation = <Date 2019-07-08.00:52:26.788>
creator = 'iomintz'
dependencies = []
files = ['48463']
hgrepos = []
issue_num = 37521
keywords = ['patch']
message_count = 16.0
messages = ['347482', '347483', '347508', '347582', '347639', '347759', '347760', '347761', '347762', '347763', '347844', '347987', '348028', '348047', '348076', '348084']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'ncoghlan', 'eric.snow', 'miss-islington', 'iomintz']
pr_nums = ['14723', '14724']
priority = 'normal'
resolution = 'fixed'
stage = 'needs patch'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue37521'
versions = ['Python 3.7']

[iomintz]

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.

[iomintz]

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'

[brettcannon]

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.)

[iomintz]

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

[brettcannon]

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

[brettcannon]

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).

[brettcannon]

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

[miss-islington]

New changeset 0827064 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)
0827064

[miss-islington]

New changeset bfb709b 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)
bfb709b

[brettcannon]

Thanks for the report, Benjamin!

[iomintz]

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

[brettcannon]

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.

[iomintz]

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

[brettcannon]

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.

[iomintz]

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.

[brettcannon]

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.