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

bpo-35486: Note Py3.6 import system API requirement change #11540

Conversation

ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Jan 13, 2019

While the introduction of ModuleNotFoundError was fully backwards
compatible on the import API consumer side, folks providing alternative
implementations of __import__ need to make an update to be
forward compatible with clients that start relying on the new subclass.

https://bugs.python.org/issue35486

@ncoghlan
Copy link
Contributor Author

ncoghlan commented Jan 13, 2019

Note that I've flagged this for Python 3.6 so that https://docs.python.org/3.6/whatsnew/3.6.html#porting-to-python-3-6 will get updated.

(cc @ned-deily)

eamanu
eamanu approved these changes Jan 13, 2019
Copy link
Contributor

@eamanu eamanu left a comment

LGTM

@ncoghlan
Copy link
Contributor Author

ncoghlan commented Jan 13, 2019

@python/import-team One thing to check here is that I think I'm right that anyone writing meta_path importers and PathFinder path hooks still doesn't need to worry about raising ModuleNotFoundError themselves, since they signal "didn't find it" by returning None in place of a loader or module spec (depending on exactly which plugin API we're talking about), and hence it's only full replacements of __import__ that need to worry about emitting ModuleNotFoundError in the appropriate places.

However, it's also possible that this change may uncover latent plugin defects where the plugin is raising ImportError directly when it should be returning None and letting the import system handle converting that to an exception if none of the available hooks produce a usable result.

@brettcannon
Copy link
Member

brettcannon commented Jan 15, 2019

If you want to be extra-thorough, the Approximating importlib.import_module() example in the importlib docs could stand to be updated to raise ModuleNotFoundError instead of ImportError.

Otherwise LGTM and your assessment that nothing in importlib as exposed publicly needs updating seems accurate (i.e. it's the loaders that will raise ImportError and none of those changed).

Doc/whatsnew/3.6.rst Outdated Show resolved Hide resolved
@ncoghlan
Copy link
Contributor Author

ncoghlan commented Jan 17, 2019

@brettcannon Example updated. It turns out that just setting name=module_name doesn't work, as it leaves the exception with a blank message.

@ncoghlan ncoghlan added the 🤖 automerge PR will be merged once it's been approved and all CI passed label Jan 17, 2019
@miss-islington
Copy link
Contributor

miss-islington commented Jan 17, 2019

@ncoghlan: Status check is done, and it's a success .

@miss-islington miss-islington merged commit cee29b4 into python:master Jan 17, 2019
@miss-islington
Copy link
Contributor

miss-islington commented Jan 17, 2019

Thanks @ncoghlan for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒🤖

@bedevere-bot
Copy link

bedevere-bot commented Jan 17, 2019

GH-11587 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 17, 2019
…11540)

While the introduction of ModuleNotFoundError was fully backwards
compatible on the import API consumer side, folks providing alternative
implementations of `__import__` need to make an update to be
forward compatible with clients that start relying on the new subclass.

https://bugs.python.org/issue35486
(cherry picked from commit cee29b4)

Co-authored-by: Nick Coghlan <ncoghlan@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 17, 2019
…11540)

While the introduction of ModuleNotFoundError was fully backwards
compatible on the import API consumer side, folks providing alternative
implementations of `__import__` need to make an update to be
forward compatible with clients that start relying on the new subclass.

https://bugs.python.org/issue35486
(cherry picked from commit cee29b4)

Co-authored-by: Nick Coghlan <ncoghlan@gmail.com>
@bedevere-bot
Copy link

bedevere-bot commented Jan 17, 2019

GH-11588 is a backport of this pull request to the 3.6 branch.

miss-islington added a commit that referenced this pull request Jan 17, 2019
While the introduction of ModuleNotFoundError was fully backwards
compatible on the import API consumer side, folks providing alternative
implementations of `__import__` need to make an update to be
forward compatible with clients that start relying on the new subclass.

https://bugs.python.org/issue35486
(cherry picked from commit cee29b4)

Co-authored-by: Nick Coghlan <ncoghlan@gmail.com>
ned-deily pushed a commit that referenced this pull request Jan 18, 2019
…GH-11588)

While the introduction of ModuleNotFoundError was fully backwards
compatible on the import API consumer side, folks providing alternative
implementations of `__import__` need to make an update to be
forward compatible with clients that start relying on the new subclass.

https://bugs.python.org/issue35486
(cherry picked from commit cee29b4)

Co-authored-by: Nick Coghlan <ncoghlan@gmail.com>
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 🤖 automerge PR will be merged once it's been approved and all CI passed skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants