msg203814 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2013-11-22 17:25 |
This is closely related to issue #19697. |
|
|
msg204187 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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) *  |
Date: 2013-12-06 07:17 |
_run_module_as_main() is particularly related to #19697. |
|
|
msg205556 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
Date: 2013-12-11 05:27 |
patch LGTM |
|
|
msg206204 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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)  |
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: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
Date: 2013-12-19 06:08 |
Thanks for working this out, Nick! |
|
|