On Sat, Nov 2, 2013 at 5:46 AM, Nick Coghlan <ncogh...@gmail.com> wrote:
> On 2 November 2013 08:44, Eric Snow <ericsnowcurren...@gmail.com> wrote: > > On Fri, Nov 1, 2013 at 11:59 AM, Brett Cannon <br...@python.org> wrote: > >> Thanks for the clarification. It all SGTM. > > > > I've updated the PEP and the reference implementation. > > While I was saying loader when I should have been saying finder, Eric > correctly divined my intent :) > > > If I've missed > > anything please let me know. There is only one thing I thought of > > that I'd like to get an opinion on: > > > > In the Finders section, the PEP specifies returning None (or using a > > different loader) when the found loader does not support loading into > > an existing module (e.g during reload). An alternative to returning > > None would be for the finder to raise ImportError with a message like > > "the loader does not support reloading the module". This may actually > > be a better approach since "could not find a loader" and "the found > > loader won't work" are different situations that a single return value > > (None) may not sufficiently represent. > > Throwing ImportError for "can load, but cannot load into that target > module" sounds good to me. SGTM as well. > We should also be explicit that throwing an > exception from find_spec *stops* the search (unlike returning None). > I thought that was obvious but more docs doesn't hurt. =) > > > Other than that, I'm not aware of any blockers for the PEP. > > The one slight quibble I have with the PEP update is the wording > regarding the "existing" parameter. I think runpy should pass the new > parameter as well when calling find_spec, but in that case it *won't* > be an existing copy of the named module, it will be > sys.modules["__main__"]. So we shouldn't set an expectation that the > existing module passed in will necessarily be a previous copy of the > module being loaded, it's just an indication to the finder that the > target namespace for execution will be a pre-existing module rather > than a new one. > > For the same reason, I also have a mild preference for "target" (or > the more explicit "load_target") as the name, although I won't object > if you and Brett prefer "existing". I would go with "module" or "target" and that is where I shall end the bikeshedding.
_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
