<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Aug 30, 2013 at 10:41 AM, Nick Coghlan <span dir="ltr"><<a href="mailto:ncoghlan@gmail.com" target="_blank">ncoghlan@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 29 August 2013 23:16, Brett Cannon <<a href="mailto:brett@python.org">brett@python.org</a>> wrote:<br>
>> Regardless, that I left this as a comment reflects my uncertainty of its<br>
>> utility.<br>
><br>
><br>
> There is lowering the barrier of entry and then there is adding a needless<br>
> API. While I appreciate wanting to make the import system more accessible,<br>
> you can't paint over sys.modules entirely, so trying to partially hide it<br>
> here won't do anyone any good when they have to deal with it in other places<br>
> (at least if they are working at the API level of needing to care if<br>
> something is loaded).<br>
<br>
</div>Agreed on this one.<br>
<div class="im"><br>
>>>> In the case where loaders already expose methods for creating<br>
>>>> and preparing modules, a finder may use ``ModuleSpec.from_module()`` on<br>
>>>> a throw-away module to create the appropriate spec.<br>
>>><br>
>>><br>
>>> Why is the module a throw-away one? And why would loaders need to<br>
>>> construct a ModuleSpec?<br>
>><br>
>><br>
>> This is something the Nick pointed out. Some loaders may already have the<br>
>> API to create a module and populate its attributes.<br>
><br>
><br>
> The key word here is "may". You simply cannot guess at needs of users<br>
> without explicit evidence this is actually actively true in the wild. Even<br>
> using importlib as an example is iffy since that's mostly my opinion of how<br>
> to do things and won't necessarily hold (e.g. the namespace classes Eric<br>
> wrote don't check their values at all while all the classes I wrote verify<br>
> their name).<br>
><br>
> Trust me, you don't want to end up supporting an API that only one person<br>
> uses. Keep this initial API small and to the point and expand it as<br>
> necessary or as requests come in for future releases. While we should aim to<br>
> get the core concepts right the first time, we can expand the API later as<br>
> necessary. There will be more Python releases after all. =)<br>
<br>
</div>Yeah, I had my sequence of events wrong when I suggested this one (I<br>
was thinking about the create/exec split when loading a module).<br>
Existing finders deal in loaders, not already constructed modules, so<br>
this doesn't actually make sense.<br>
<br>
Let's make it go away :)<br>
<div class="im"><br>
>> As with create_module(), Loader.exec_module() isn't in charge of setting<br>
>> the import-related attributes (as opposed to Loader.load_module(), which<br>
>> is). However, if the module was replaced in sys.modules during<br>
>> exec_module(), then we assume that the one in sys.modules has not been<br>
>> touched by the spec. So we set any of the import-related attributes on it<br>
>> that aren't already set (respecting the ones that are), with the exception<br>
>> of __spec__, which we always override.<br>
>><br>
>> Thinking about it, it may make sense in that case to create a new spec<br>
>> based on the current one but deferring to any of the existing<br>
>> import-related attributes of the module. Then that spec can be set to<br>
>> __spec__.<br>
><br>
><br>
> If you do that then you are starting to shift to either a loader method or a<br>
> function somewhere in importlib, else you are creating a conditional factory<br>
> for specs.<br>
<br>
</div>I still like the idea of keeping __spec__ as "this is how we found it<br>
originally", with only the module attributes reflecting any dynamic<br>
updates.<br>
<div class="im"><br>
>>> Since exec_module() is going to need to set these anyway for proper<br>
>>> exec() use during loading then why are you setting them *again* later on?<br>
>>> Should you set these first and then let the methods reset them as they see<br>
>>> fit? I thought exec_module() took in a filled-in module anyway, so didn't<br>
>>> you have to set all the attributes prior to passing it in anyway in step<br>
>>> 1.a? In that case this is a reset which seems wrong if code explicitly chose<br>
>>> to change the values.<br>
>><br>
>><br>
>> It's not that code chose to change the values. It's that code chose to<br>
>> stick some other object in sys.modules and we're going to return it and we<br>
>> want to be sure the spec and import-related attributes are all properly set.<br>
><br>
><br>
> Do we though? We do that currently except for __package__ and __loader__,<br>
> but that's for backwards-compatibility since those attributes are so new.<br>
> This also destroys the possibility of lazy loading, btw (which I<br>
> unfortunately realized after 3.3 was released, so I kind of regret it).<br>
><br>
> Since the attributes have to be set before any exec() call I understand<br>
> making sure they are set at that point, but if you're going to shove some<br>
> other object in sys.modules then I view it as your problem to make sure you<br>
> set those attributes as you are already deviating from normal import<br>
> semantics. IOW if you are given a module you can and should expect it is as<br>
> prepared as the import system can make it to be executed on, but if you<br>
> choose to ignore it then you are on your own.<br>
<br>
</div>+1 for leaving the object in sys.modules alone after we call exec_module.<br>
<div class="im"><br>
> You've abstracted out what method is called, which should mean I don't have<br>
> to care which method is used (in case both are defined by a very compatible<br>
> loader). But in this case I do are as one will cache its results and the<br>
> other won't. Since sys.modules is such a core data structure you should<br>
> always know when it has been tampered with, but these semantics make it<br>
> unclear based on the the arguments used as to what the result will be in<br>
> regards to implicit side-effects.<br>
><br>
> I'm going to argue that you should have unified side-effects in regards to<br>
> sys.modules so exec_module() should set sys.modules if it was not done so by<br>
> the loader. If you want you can add a keyword-only argument to make that<br>
> "optional" (i.e. explicitly remove from sys.modules or just don't explicit<br>
> set it but whatever the loader did it did) or vice-versa, but there should<br>
> be some way for me to be able to say "I don't want any surprises in regards<br>
> to sys.modules, so be consistent regardless of loader method called".<br>
<br>
</div>No, the new methods are *supposed* to be stateless with no global side<br>
effects, leaving all import state manipulation to the import system.<br>
We just can't *promise* no side effects for exec_module, since the<br>
module code might itself mess with the process global state. The fact<br>
load_module is *expected* to mess with sys.modules is a design flaw<br>
that shouldn't be perpetuated.<br></blockquote><div><br></div><div>OK, then that needs to be clearly specified in the PEP that the fact that occurs is a design flaw that will slowly go away as time goes on and people move to the new API.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
What I would really like is to be able to call<br>
"target_loader.exec_module(existing_main_module)" from runpy,<br>
*without* calling create_module first.<br>
<br>
And now I remember why I wanted to be able to pass an existing module<br>
to a loader and say "hey, can you use this as an execution<br>
namespace?": so I could pass them __main__, instead of having to<br>
hardcode knowledge of different kinds of loader into runpy.<br>
<br>
So perhaps a better name might be "prepare_module" (by analogy to PEP<br>
3115), and have it accept a "reloading" parameter, which is an<br>
existing module to be reused.<br></blockquote><div><br></div><div>Is this to replace create_module() or exec_module()?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The signature would be something like:<br>
<br>
def prepare_module(reloading=None):<br>
"""Create a module object for execution. Returning None will<br>
created a default module.<br></blockquote><div><br></div><div>I can't follow that sentence. =) What does returning None represent?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If *reloading* is set, specifies an existing sys.modules entry<br>
that is being reloaded.</blockquote><div><br></div><div>As in the key into sys.modules?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Must return None or that<br>
specific object if reloading is supported.</blockquote><div><br></div><div>What's "that" supposed to represent?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Returning a<br>
different module object or explicitly raising ImportError<br>
indicates that reloading is not supported. (Or perhaps define<br>
a "ReloadError" subclass of ImportError?)<br>
<div class="im"> """<br></div></blockquote><div><br></div><div>I'm really not following what this method is supposed to do. Is it simply mucking with sys.modules? Is it creating a module to use? If it's the latter then how does return None do anything? Are you saying returning None means "I didn't do anything special, do what you want"?</div>
<div><br></div><div>-Brett</div></div></div></div>