<div dir="ltr">On Wed, Sep 18, 2013 at 10:08 AM, Nick Coghlan <span dir="ltr"><<a href="mailto:ncoghlan@gmail.com" target="_blank">ncoghlan@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On 18 September 2013 19:51, Eric Snow <<a href="mailto:ericsnowcurrently@gmail.com">ericsnowcurrently@gmail.com</a>> wrote:<br>
> Hi all,<br>
><br>
> I finally got some time to update the PEP. I've simplified a few things,<br>
> most notably by making the 4 ModuleSpec methods (create, exec, load, reload)<br>
> "private".<br>
><br>
> Also notable is that the new loader method is still create_module() and<br>
> there is still no flag for is_reload on either of the loader methods. I'm<br>
> still not clear on what the flag buys us and on why anything we'd do in a<br>
> prepare_module() we couldn't do in exec_module(). I'm trying to keep this<br>
> simple. :)<br>
<br>
</div>The point is to give the invoker of the loader a chance to muck about<br>
with the module state before actually executing the module. For<br>
example, runpy and the updated extension loader API could use this to<br>
support execution of compiled Cython modules with -m.<br></blockquote><div><br></div><div>That makes sense. A loader.create_module() method (not called during reload) gives you that. I'm all for that. I'm just not clear on why it needs to be more than that.</div>
<div><br></div><div>My understanding of the proposed prepare_module() is it would always be called right before exec_module(), whether it be load or reload (there would be no create_module()). Then in that case, can't loaders just roll their prepare_module() implementation into the beginning of exec_module() (even call spec.init_module_attrs() directly)? What's the advantage to splitting that out in the Loader API? I know I'm missing something here. (Maybe I shouldn't try to work on the PEP so late at night!)</div>
<div><br></div><div>...after further consideration...</div><div><br></div><div>I expect it's so that during reload the loader can indicate "don't reload in-place, load into this module instead!" So the module passed in to exec_module() would end up being different from the existing module in sys.modules. However, can't exec_module() simply exec into the module that it would have returned from prepare_module() and then directly stick it into sys.modules?</div>
<div><br></div><div><div>...after further consideration...</div><div><br></div></div><div>Okay, maybe I'm seeing it. Would it be something like the following?</div><div><br></div><div>#-- start prepare_module() example --<br>
</div><div><br></div><div>class ModuleSpec:</div><div> ...</div><div> def _load(self):</div><div> # This is basically the same as the PEP currently defines it.</div><div> module = self.loader.prepare_module(self) # I prefer create_module for this.</div>
<div> if module is None:</div><div> module = ModuleType(<a href="http://self.name">self.name</a>)</div><div> self.init_module_attrs(module)</div><div> # skipping some boilerplate</div><div>
sys.modules[<a href="http://self.name">self.name</a>] = module</div><div> self.loader.exec_module(module)</div><div> return sys.modules[<a href="http://self.name">self.name</a>]</div><div><br></div><div>
def _reload(self, module):</div><div> # This is where it gets different.</div><div> prepared = self.loader.prepare_module(self, module)</div><div> if prepared is not None:</div><div> self.init_module_attrs(prepared)</div>
<div> module = prepared</div><div> sys.modules[<a href="http://self.name">self.name</a>] = module</div><div> self.loader.exec_module(module)</div><div> return sys.modules[<a href="http://self.name">self.name</a>]</div>
<div><br></div><div>class SomeLoader:</div><div><br></div><div> def prepare_module(self, spec, module=None):</div><div> if self.never_ever_been_loaded_before_not_even_in_subinterpreters(<a href="http://spec.name">spec.name</a>):<br>
</div><div> self.initialize_stuff(spec)</div><div> return MyCustomModule(<a href="http://spec.name">spec.name</a>)</div><div><br></div><div> def exec_module(self, module):</div><div> # Do exec stuff here.</div>
<div><br></div><div>#-- end prepare_module() example --</div><div><br></div><div>(Note that _load() and _reload() could share more code than they do, but regardless...)</div><div><br></div><div>Contrast that with what the PEP specifies currently.<br>
</div><div><br></div><div><div>#-- start current PEP example --<br></div></div><div><br></div><div><div>class ModuleSpec:</div><div> ...</div><div> def _create(self):</div><div> module = self.loader.create_module(self)</div>
<div> if module is None:</div><div> module = ModuleType(<a href="http://self.name">self.name</a>)</div><div> self.init_module_attrs(module)</div><div> return module</div><div><br></div><div>
def _load(self):</div><div> module = self._create()</div><div> # skipping boilerplate<br></div><div> self.loader.exec_module(module)</div><div> return sys.modules[<a href="http://self.name">self.name</a>]</div>
<div><br></div><div> def _reload(self, module):</div><div> self.loader.exec_module(module)<br></div><div> return sys.modules[<a href="http://self.name">self.name</a>]</div></div><div><br></div><div>class SomeLoader:<br>
</div><div><br></div><div> def create_module(self, spec):</div><div> if self.never_ever_been_loaded_before_not_even_in_subinterpreters(<a href="http://spec.name">spec.name</a>):<br></div><div> self.initialize_stuff(spec)</div>
<div><div> return MyCustomModule(<a href="http://spec.name">spec.name</a>)</div></div><div><br></div><div> def exec_module(self, module):</div><div> if not self.never_ever_been_loaded_before_not_even_in_subinterpreters(<a href="http://spec.name">spec.name</a>):<br>
</div><div> module = module.__spec__._create()</div><div> # or module = self.create_module(spec); spec.init_module_attrs(module)</div><div> sys.modules[module.__name__] = module</div><div>
# Do exec stuff here.</div><div><div><br></div><div>#-- end current PEP example --<br></div></div><div><br></div><div>The way I see it, in the latter example the ModuleSpec is easier to follow, without making exec_module() that much more complicated.</div>
<div><br></div><div>Regardless, at this point I'm seeing prepare_module() as a formal API for "use *this* module instead of what you would use by default." While create_module() provides that for the loading case, prepare_module() also provides it explicitly for the reloading case. Consequently, in the reload case prepare_module() does eliminate the boilerplate that exec_module() otherwise must accommodate. That's probably the biggest reason to go there.</div>
<div><br></div><div>I wonder if we could instead wrap that bit in a ModuleSpec helper method that loaders can call in exec_module():</div><div><br></div><div> def _new_module_for_reload(self):</div><div> module = self._create()</div>
<div> sys.modules[<a href="http://self.name">self.name</a>] = module</div><div><br></div><div>FWIW, I think create_module() is still an appropriate (and better) name regardless of where it's used.</div><div><br></div>
<div>At this point I still would rather stick with what the PEP currently specifies, but I'm going ruminate on the reload case--e,g, re-read your message about reload strategies as well as your response to my message about module lifecycles. I think I have a more context to fit them into the big picture here.</div>
<div><br></div><div>Not to leave anything out, is there any reason we shouldn't punt right now on the whole reload mechanics issue and bundle it with the PEP on improving extension modules? I'd like to wrap up ModuleSpec and see about the .ref PEP that started all this. Plus I think this PEP is hitting the limit of a mentally bite-size proposal. I've been lamentably busy of late so I'm worried about expanding them PEP. However, I'm open to more discussion on supporting other reload strategies, particularly if you think this PEP should not move forward with having settled the issue.</div>
<div><br></div><div>BTW, thanks for diving into the extension module questions (you and Stefan). Those discussions have helped improve this PEP. :) </div></div><br></div><div class="gmail_extra">-eric</div></div>