On 27 October 2013 14:29, Eric Snow <ericsnowcurren...@gmail.com> wrote:
> On Sat, Oct 26, 2013 at 9:44 PM, Nick Coghlan <ncogh...@gmail.com> wrote:
>> Accordingly, I think we should add a "How Reloading Will Work"
>> section, akin to the existing "How Loading Will Work"
>> (http://www.python.org/dev/peps/pep-0451/#how-loading-will-work). We
>> may also want to spit out some import warnings for edge cases that are
>> likely to do the wrong thing.
>
> I'm tempted to just say reload should not be a blocker for PEP 451.
> The premise is that the PEP mostly maintains the status quo, just
> shifting reload-based-on-loader to reload-based-on-spec (it's still
> "*load* the same way you did the first time").
>
> I agree that it would be worth getting reload working right, but I'm
> not convinced it's worth it to delay PEP 451 more for the sake of
> reload.

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

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]

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
_______________________________________________
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

Reply via email to