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

docs claim __import__ checked for in globals, but IMPORT_NAME bytecode does not #69686

Closed
superbobry mannequin opened this issue Oct 28, 2015 · 5 comments
Closed

docs claim __import__ checked for in globals, but IMPORT_NAME bytecode does not #69686

superbobry mannequin opened this issue Oct 28, 2015 · 5 comments
Assignees
Labels
docs Documentation in the Doc dir stdlib Python modules in the Lib dir

Comments

@superbobry
Copy link
Mannequin

superbobry mannequin commented Oct 28, 2015

BPO 25500
Nosy @brettcannon, @bitdancer, @ericsnowcurrently, @superbobry

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 2015-12-04.22:54:12.566>
created_at = <Date 2015-10-28.21:34:44.557>
labels = ['library', 'docs']
title = 'docs claim __import__ checked for in globals, but IMPORT_NAME bytecode does not'
updated_at = <Date 2015-12-04.22:54:12.565>
user = 'https://github.com/superbobry'

bugs.python.org fields:

activity = <Date 2015-12-04.22:54:12.565>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2015-12-04.22:54:12.566>
closer = 'brett.cannon'
components = ['Documentation', 'Library (Lib)']
creation = <Date 2015-10-28.21:34:44.557>
creator = 'superbobry'
dependencies = []
files = []
hgrepos = []
issue_num = 25500
keywords = []
message_count = 5.0
messages = ['253635', '253638', '253642', '255891', '255892']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'r.david.murray', 'docs@python', 'python-dev', 'eric.snow', 'superbobry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25500'
versions = ['Python 3.4', 'Python 3.5']

@superbobry
Copy link
Mannequin Author

superbobry mannequin commented Oct 28, 2015

According to the current import system documentation

When calling __import__() as part of an import statement, the import system first checks the module global namespace for a function by that name. If it is not found, then the standard builtin __import__() is called.

However, one can easily verify this isn't (always) the case::

    import sys
assert "glob" not in sys.modules
\_\_import__ = print

import glob  # Doesn't print anything.

I've traced the import statement from ceval.c to the frozen importlib._bootstrap and it seems the cause of the problem is in _find_and_load_unlocked, which simply ignores the _import argument in the case above::

    def _find_and_load_unlocked(name, import_):
        path = None
        # ... parent processing ...
        spec = _find_spec(name, path)
        if spec is None:
            raise ImportError(_ERR_MSG.format(name), name=name)
        else:
            # XXX import_ is not used.
            module = _SpecMethods(spec)._load_unlocked()
        # ... more parent processing ...
        return module

I'm not sure if this is a bug in the documentation or implementation, so any feedback is appreciated.

@superbobry superbobry mannequin assigned docspython Oct 28, 2015
@superbobry superbobry mannequin added docs Documentation in the Doc dir stdlib Python modules in the Lib dir labels Oct 28, 2015
@brettcannon
Copy link
Member

I think the documentation is wrong. Going all the way back to Python 2.7, you will see that importing a module emits IMPORT_NAME as the bytecode. That bytecode only looks for __import__ in the builtin namespace: https://hg.python.org/cpython/file/2.7/Python/ceval.c#l2588 . That then calls PyImport_ImportModuleLevel(): https://hg.python.org/cpython/file/2.7/Python/bltinmodule.c#l49 . That then just ends up executing import.

If you look at PyImport_Import() in Python 2.7 that comes the closest to what the docs reference, but that still uses __builtins__.__import__ based on finding __builtins__ from the global namespace: https://hg.python.org/cpython/file/2.7/Python/import.c#l2825 . But that isn't directly called by import itself and is just a C-level API.

If you look in Python 3, then you will see that as far back as Python 3.3 the code in importlib is more or less the same: __import__ is used when importing a parent and then importlib is used for the rest: https://hg.python.org/cpython/file/default/Lib/importlib/_bootstrap.py#l944 . This was introduced so that the accelerated C version of __import__ would be used to import the parent rather than automatically going down the pure Python path for stuff such as resolving names and such. This was never done to explicitly import using something defined in the globals namespace (although this does lead to supporting it).

And if you go all the way back to Python 3.0 you will notice that getting __import__ from the builtins namespace only has been in place: https://hg.python.org/cpython/file/3.0/Python/ceval.c#l1959 .

So I think the key point here is that the bytecode for IMPORT_NAME doesn't match the docs. So the question is do we want to add the feature to Python 3 to look for import in the globals or would we rather leave it as is and fix the docs?

@brettcannon brettcannon changed the title _find_and_load_unlocked doesn't always use __import__ docs claim __import__ checked for in globals, but IMPORT_NAME bytecode does not Oct 28, 2015
@bitdancer
Copy link
Member

Fix the docs.

@brettcannon brettcannon assigned brettcannon and unassigned docspython Dec 2, 2015
@python-dev
Copy link
Mannequin

python-dev mannequin commented Dec 4, 2015

New changeset 567baf74ebad by Brett Cannon in branch '3.5':
Issue bpo-25500: Fix the language reference to not claim that import
https://hg.python.org/cpython/rev/567baf74ebad

New changeset 0259c2c555fb by Brett Cannon in branch 'default':
Merge for issue bpo-25500
https://hg.python.org/cpython/rev/0259c2c555fb

@brettcannon
Copy link
Member

Thanks for the bug report, Sergei! I fixed the docs in the end.

@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
docs Documentation in the Doc dir stdlib Python modules in the Lib dir
Projects
None yet
Development

No branches or pull requests

2 participants