On Sat, Oct 26, 2013 at 11:03 PM, Nick Coghlan <ncogh...@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
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
