[Python-Dev] PEP 451 update

[Nick Coghlan]

On 29 Oct 2013 14:45, "Eric Snow" <ericsnowcurrently at gmail.com> wrote:
>
> On Sat, Oct 26, 2013 at 11:03 PM, Nick Coghlan <ncoghlan at gmail.com> wrote:
> > I don't think we can postpone it to later, as we need to be clear on:
> >
> > 1. Does reload use __name__ or __spec__.name when both are available?
> > 2. Does __name__ get restored to its original value if reloading via
> > __spec__.name?
> > 3. Do other import related attributes get restored to their original
values?
> > 4. Does create_module get called if the loader has an exec_module
method?
> > 5. Does the loader have access to the previous spec when reloading a
module?
> >
> > My answers to these questions (note that my answer to 2 differs from
> > what I had in my initial sketch):
> >
> > 1. __spec__.name
> > 2. Yes, __name__ is updated to match __spec__.name, *expect* if it is
> > currently "__main__"
> > 3. Yes, other import related attributes are restored to their baseline
values
> > 4. No, create_module is never called when reloading
>
> I agree on all of these.  I'm adding a "How reloading will work"
> section to the PEP.
>
> > 5. Currently no, but I'm thinking that may be worth changing (more on
> > that below)
> >
> > The reload() implementation in my message is actually based on the
> > current importlib.reload implementation. The only PEP 451 related
> > changes were:
> >
> > - using __spec__.name (if available) instead of __name__
> > - checking all parent modules exist rather than just the immediate
parent module
> > - calling import.find_spec() rather than using the __loader__ attribute
directly
> > - being explicit that __name__ is left at the value it had prior to the
reload
> > - handling the namespace package, exec_module and no exec_module cases
> >
> > I also added an explicit check that the module was re-used properly,
> > but I agree *that* change is out of scope for the PEP and should be
> > considered as a separate question.
> >
> > Now, regarding the signature of exec_module(): I'm back to believing
> > that loaders should receive a clear indication that a reload is taking
> > place. Legacy loaders have to figure that out for themselves (by
> > seeing that the module already exists in sys.modules), but we can do
> > better for the new API by making the exec_module signature look like:
> >
> >     def exec_module(self, module, previous_spec=None):
> >         # module is as per the current PEP 451 text
> >         # previous_spec would be set *only* in the reload() case
> >         # loaders that don't care still need to accept it, but can
> > just ignore it
> >         ...
>
> I'd rather give loaders another optional method:
> supports_reload(name).  Complicating the spec methods signatures (to
> support passing the old spec through) just for the sake of reload
> seems less than optimal.  I don't see any benefit to exposing the old
> spec to loader.exec_module().

I do: it lets the loader tell if anything changed from the previous load
operation, allowing it to make an informed decision on whether or not to
permit the reload operation or throw an exception.

However, I accept such a check would be better implemented as a separate
yes/no method, so it makes sense to postpone this aspect to 3.5 at the
earliest.

Cheers,
Nick.

>
> >
> > So, with those revisions, the reload() semantics would be defined as:
> >
> >     def reload(module):
> >         # Get the name to reload from the spec if it is available,
> >         # otherwise use __name__ directly
> >         current_spec = getattr(module, "__spec__")
> >         current_name = module.__name__
> >         name_to_reload = current_name if current_spec is None else
> > current_spec.name
> >
> >         # Check this module is properly imported before trying to
reload it
> >         loaded_module = sys.modules.get(name_to_reload)
> >         if loaded_module is not module:
> >             msg = "module {} not in sys.modules"
> >             raise ImportError(msg.format(name_to_reload),
name=name_to_reload)
> >         parent_name = name_to_reload.rpartition('.')[0]
> >         while parent_name:
> >             if parent_name not in sys.modules:
> >                 msg = "parent {!r} not in sys.modules"
> >                 raise ImportError(msg.format(parent_name),
name=parent_name)
> >             parent_name = parent_name.rpartition('.')[0]
> >
> >         # Check for a modified spec (e.g. if the import hooks have
changed)
> >         updated_spec = importlib.find_spec(name_to_reload)
> >
> >         # The import-related module attributes get updated here
> >         _init_module_attrs(loaded_module, updated_spec)
> >         if current_name == "__main__":
> >             loaded_module.__name__ = "__main__"
> >
> >         # Now re-execute the module
> >         loader = updated_spec.loader
> >         if loader is None:
> >             # Namespace package, already reinitialised above
> >             return loaded_module
> >         elif hasattr(loader, 'exec_module'):
> >             loader.exec_module(module, current_spec)
> >         else:
> >             loader.load_module(name_to_reload)
> >
> >         # Allow for the module to be replaced in sys.modules
> >         # (even though that likely won't work very well)
> >         return sys.modules[name_to_reload]
>
> This is pretty much the same thing I've implemented. :)
>
> -eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20131029/4ed4b2f7/attachment-0001.html>