Re: PEP 451: Big update.

On Wed, Sep 18, 2013 at 10:08 AM, Nick Coghlan <[email protected]> wrote:

> On 18 September 2013 19:51, Eric Snow <[email protected]> wrote:
> > Hi all,
> >
> > I finally got some time to update the PEP.  I've simplified a few things,
> > most notably by making the 4 ModuleSpec methods (create, exec, load,
> reload)
> > "private".
> >
> > Also notable is that the new loader method is still create_module() and
> > there is still no flag for is_reload on either of the loader methods.
>  I'm
> > still not clear on what the flag buys us and on why anything we'd do in a
> > prepare_module() we couldn't do in exec_module().  I'm trying to keep
> this
> > simple. :)
>
> The point is to give the invoker of the loader a chance to muck about
> with the module state before actually executing the module. For
> example, runpy and the updated extension loader API could use this to
> support execution of compiled Cython modules with -m.
>

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.

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!)

...after further consideration...

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?

...after further consideration...

Okay, maybe I'm seeing it.  Would it be something like the following?

#-- start prepare_module() example --

class ModuleSpec:
    ...
    def _load(self):
        # This is basically the same as the PEP currently defines it.
        module = self.loader.prepare_module(self)  # I prefer create_module
for this.
        if module is None:
            module = ModuleType(self.name)
        self.init_module_attrs(module)
        # skipping some boilerplate
        sys.modules[self.name] = module
        self.loader.exec_module(module)
        return sys.modules[self.name]

    def _reload(self, module):
        # This is where it gets different.
        prepared = self.loader.prepare_module(self, module)
        if prepared is not None:
            self.init_module_attrs(prepared)
            module = prepared
            sys.modules[self.name] = module
        self.loader.exec_module(module)
        return sys.modules[self.name]

class SomeLoader:

    def prepare_module(self, spec, module=None):
        if self.never_ever_been_loaded_before_not_even_in_subinterpreters(
spec.name):
            self.initialize_stuff(spec)
        return MyCustomModule(spec.name)

    def exec_module(self, module):
        # Do exec stuff here.

#-- end prepare_module() example --

(Note that _load() and _reload() could share more code than they do, but
regardless...)

Contrast that with what the PEP specifies currently.

#-- start current PEP example --

class ModuleSpec:
    ...
    def _create(self):
        module = self.loader.create_module(self)
        if module is None:
            module = ModuleType(self.name)
        self.init_module_attrs(module)
        return module

    def _load(self):
        module = self._create()
        # skipping boilerplate
        self.loader.exec_module(module)
        return sys.modules[self.name]

    def _reload(self, module):
        self.loader.exec_module(module)
        return sys.modules[self.name]

class SomeLoader:

    def create_module(self, spec):
        if self.never_ever_been_loaded_before_not_even_in_subinterpreters(
spec.name):
            self.initialize_stuff(spec)
        return MyCustomModule(spec.name)

    def exec_module(self, module):
        if not
self.never_ever_been_loaded_before_not_even_in_subinterpreters(spec.name):
            module = module.__spec__._create()
            # or module = self.create_module(spec);
spec.init_module_attrs(module)
            sys.modules[module.__name__] = module
        # Do exec stuff here.

#-- end current PEP example --

The way I see it, in the latter example the ModuleSpec is easier to follow,
without making exec_module() that much more complicated.

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.

I wonder if we could instead wrap that bit in a ModuleSpec helper method
that loaders can call in exec_module():

  def _new_module_for_reload(self):
      module = self._create()
      sys.modules[self.name] = module

FWIW, I think create_module() is still an appropriate (and better) name
regardless of where it's used.

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.

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.

BTW, thanks for diving into the extension module questions (you and
Stefan).  Those discussions have helped improve this PEP. :)

-eric

_______________________________________________
Import-SIG mailing list
[email protected]
https://mail.python.org/mailman/listinfo/import-sig