Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
iomintz mannequin opened this issue Jul 8, 2019 · 16 comments
Closed

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

iomintz mannequin opened this issue Jul 8, 2019 · 16 comments
Assignees
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir

Comments

@iomintz
Copy link
Mannequin

iomintz mannequin commented Jul 8, 2019

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
    Copy link
    Mannequin Author

    iomintz mannequin commented Jul 8, 2019

    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 iomintz mannequin added stdlib Python modules in the Lib dir 3.7 (EOL) end of life labels Jul 8, 2019
    @iomintz
    Copy link
    Mannequin Author

    iomintz mannequin commented Jul 8, 2019

    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
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    iomintz mannequin commented Jul 9, 2019

    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
    Copy link
    Member

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

    @brettcannon brettcannon self-assigned this Jul 10, 2019
    @brettcannon
    Copy link
    Member

    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 brettcannon added docs Documentation in the Doc dir and removed stdlib Python modules in the Lib dir labels Jul 12, 2019
    @brettcannon brettcannon changed the title importlib.util.module_from_spec return value is not the same as in sys.modules importlib examples have their exec_module()/sys.modules assignment lines reversed Jul 12, 2019
    @brettcannon
    Copy link
    Member

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

    @miss-islington
    Copy link
    Contributor

    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
    Copy link
    Contributor

    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
    Copy link
    Member

    Thanks for the report, Benjamin!

    @iomintz
    Copy link
    Mannequin Author

    iomintz mannequin commented Jul 13, 2019

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

    @brettcannon
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    iomintz mannequin commented Jul 16, 2019

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

    @brettcannon
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    iomintz mannequin commented Jul 17, 2019

    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
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants