classification
Title: Update runpy for PEP 451
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Arfrever, brett.cannon, eric.snow, larry, ncoghlan, python-dev
Priority: normal Keywords: patch

Created on 2013-11-22 16:31 by brett.cannon, last changed 2013-12-19 06:08 by eric.snow. This issue is now closed.

Files
File name Uploaded Description Edit
issue19700_runpy_spec.diff ncoghlan, 2013-12-08 13:59 First draft at spec based runpy review
issue19700_runpy_spec_v2.diff ncoghlan, 2013-12-14 23:54 Fixed namespace package handling review
issue19700_runpy_spec_v3.diff ncoghlan, 2013-12-15 01:01 Also checks specs, fails on the zipimport loader checks. review
issue19700_runpy_spec_v4.diff ncoghlan, 2013-12-15 01:15 Skip checking the loader for the zipimport case review
issue19700_runpy_spec_v5.diff ncoghlan, 2013-12-15 02:08 Partial docs update, broken "script without suffix" test case review
issue19700_runpy_spec_v6.diff ncoghlan, 2013-12-15 10:24 Final version with docs updates and no-suffix compatibility review
Messages (17)
msg203814 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-11-22 17:25
This is closely related to issue #19697.
msg204187 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-24 07:33
D'oh, I forgot there was a runpy API change I planned to offer as part of the PEP 451 integration: exposing a "target" parameter in both run_path and run_module.

http://www.python.org/dev/peps/pep-0451/#the-target-parameter-of-find-spec

I guess that slips to 3.5 now, since there's no way it will be in for feature freeze :(
msg205356 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-12-06 07:17
_run_module_as_main() is particularly related to #19697.
msg205556 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-08 13:09
OK, I just noticed that importlib.find_spec copies the annoying-as-hell behaviour of importlib.find_loader where it doesn't work with dotted names. Can we fix that please? It's stupid that pkgutil.get_loader has to exist to workaround that design flaw for find_loader, and I'd hate to see it replicated in a new public facing API.
msg205563 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-08 13:59
Deleted a bunch of code, and runpy now correctly sets both __file__ and __cached__ (runpy previously never set the latter properly).

You can also see the reason for my rant above in the form of runpy._fixed_find_spec. importlib.find_loader was always kind of useless in end user code that needed to handle arbitrary modules, you needed to use the pkgutil.get_loader wrapper instead. It would be nice if importlib.find_spec "just worked", instead of only working for top level modules. At the very least, it should fail noisily if a dotted name is passed in without specifying a path argument.

I may have argued in favour of the side effect free find_loader in the past, if I have, consider this me admitting I was wrong after wasting a bunch of time hunting down unexpected import errors in test_runpy after the initial conversion to using find_spec.

There's one final change needed to address the pickle-in-main compatibility issue: runpy has to alias the module under its real name as well as __main__. Once it does that, then using __spec__.name (when available) should ensure pickle does the right thing.
msg205579 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-12-08 16:18
Can you file a separate bug, Nick, for the importlib.find_spec() change you are after? I don't want to lose track of it (and I get what you are after but we should discuss why importlib.find_loader() was designed the way it was initially).
msg205806 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-10 12:30
Issue 19944 now covers moving the current importlib.find_spec to importlib.find_spec_on_path and having importlib.find_spec behave like runpy._fixed_find_spec in my patch.
msg205882 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-12-11 05:27
patch LGTM
msg206204 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-14 23:54
I added some unit tests for the interactions between runpy and namespace packages, which showed that I was doing the check for __main__ submodules and the check for "no loader" in the wrong order.

Last missing piece is to ensure that __spec__ is being populated appropriately, then I'll check this in.
msg206207 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-15 01:01
So close!

importlib.util.spec_from_file_location failed me (unsurprisingly) when it came to the zipfile execution tests. I'm thinking I'll just hack in a way to avoid checking the loader when the expected loader is set to None.
msg206208 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-15 01:15
Latest version simply doesn't check the loader type for the zipimport tests. (Note that just using find_spec doesn't work, since the directory and zipfile execution tests include implicit sys.path manipulation)

I also removed the separated "precompiled" flag that was in earlier patches, since importlib.util.spec_from_file_location deals with that for us.

If the full regression test suite passes, I'll be checking this version in.
msg206210 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-15 02:08
Working on the docs updates made me realise the test cases didn't cover the "no suffix" case that is causing grief in issue 19946.

So I've added a test case for that now, but haven't fixed it yet (will need to deal with the __spec__ = None case for such scripts)
msg206221 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-15 10:24
Final patch that reflects the version I'm about to commit. It includes appropriate docs updates, and ensures __main__.__spec__ is None in the cases where the import system isn't involved in initialising main.
msg206222 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-12-15 10:33
New changeset 51dddfead80a by Nick Coghlan in branch 'default':
Issue #19700: set __spec__ appropriately in runpy
http://hg.python.org/cpython/rev/51dddfead80a
msg206223 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-15 10:35
Final review of the patch showed it *wasn't* quite done, it's still missing the "both __name__ and __spec__.name are configured in sys.modules" change that is needed to get more pickle friendly behaviour from __main__.

However, I wanted to commit this version to help unblock issue 19946.
msg206425 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-12-17 12:30
On second thoughts, I'm going to close this one - if further runpy changes are needed to resolve issue 19702 (using __spec__.name for pickle when appropriate), let's deal with them there.
msg206581 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-12-19 06:08
Thanks for working this out, Nick!
History
Date User Action Args
2013-12-19 06:08:24eric.snowsetmessages: + msg206581
2013-12-17 12:30:37ncoghlansetstatus: open -> closed
type: enhancement
messages: + msg206425

resolution: fixed
stage: test needed -> resolved
2013-12-15 10:36:46ncoghlanunlinkissue19946 dependencies
2013-12-15 10:35:52ncoghlansetmessages: + msg206223
2013-12-15 10:33:21python-devsetnosy: + python-dev
messages: + msg206222
2013-12-15 10:24:05ncoghlansetfiles: + issue19700_runpy_spec_v6.diff

messages: + msg206221
2013-12-15 02:08:09ncoghlansetfiles: + issue19700_runpy_spec_v5.diff

messages: + msg206210
2013-12-15 01:49:28ncoghlanlinkissue19944 dependencies
2013-12-15 01:16:05ncoghlansetfiles: + issue19700_runpy_spec_v4.diff

messages: + msg206208
2013-12-15 01:01:29ncoghlansetfiles: + issue19700_runpy_spec_v3.diff

messages: + msg206207
2013-12-14 23:55:01ncoghlansetfiles: + issue19700_runpy_spec_v2.diff

messages: + msg206204
2013-12-14 13:22:13ncoghlanlinkissue19982 dependencies
2013-12-14 13:17:37ncoghlanlinkissue19946 dependencies
2013-12-11 05:27:52eric.snowsetmessages: + msg205882
2013-12-10 12:30:36ncoghlansetmessages: + msg205806
2013-12-09 02:04:40ncoghlanlinkissue15403 dependencies
2013-12-08 16:18:29brett.cannonsetmessages: + msg205579
2013-12-08 13:59:55ncoghlansetfiles: + issue19700_runpy_spec.diff
keywords: + patch
messages: + msg205563
2013-12-08 13:09:48ncoghlansetmessages: + msg205556
2013-12-08 11:43:54ncoghlansetassignee: ncoghlan
2013-12-06 07:17:51eric.snowsetmessages: + msg205356
2013-12-06 05:47:53eric.snowlinkissue19697 dependencies
2013-11-29 16:01:49brett.cannonlinkissue19702 dependencies
2013-11-24 07:33:52ncoghlansetnosy: + larry
messages: + msg204187
2013-11-23 19:50:59Arfreversetnosy: + Arfrever
2013-11-22 17:25:25eric.snowsetmessages: + msg203814
2013-11-22 16:32:13brett.cannonlinkissue18864 dependencies
2013-11-22 16:31:23brett.cannoncreate