[Python-Dev] PEP 451 update

[Eric Snow]

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().

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