On 27 October 2013 03:27, Nick Coghlan <ncogh...@gmail.com> wrote: > On 26 October 2013 08:51, PJ Eby <p...@telecommunity.com> wrote: >> Mostly, this just seems like an ugly wart -- Python should be dynamic >> by default, and that includes reloading. While the import machinery >> has lots of ugly caching under the hood, a user-level function like >> reload() should not require you to do the equivalent of saying, "no, >> really... I want you to *really* reload, not just pull in whatever >> exists where you found it last time, while ignoring whether I switched >> from module to package or vice versa, or just fixed my sys.path so I >> can load the right version of the module." >> >> It is a really tiny thing in the overall scheme of things, because >> reload() is not used all that often, but it's still a thing. If this >> isn't treated as a bug, then the docs for reload() at least need to >> include a forward-supported workaround so you can say "no, really... >> *really* reload" in an approved fashion. >> >> (ISTM that any production code out there that currently uses reload() >> would want to perform the "really reload" incantation in order to >> avoid the edge cases, even if they haven't actually run into any of >> them yet.) > > Having imp.reload() kick off the full reload cycle makes sense to me. > The only reason it doesn't currently do that in importlib is because > there was no test for it in the regression test suite, and PEP 302 and > the library reference are entirely vague on how module reloading works > (aside from the requirement that it reuse the same module namespace).
I've been thinking about this, and I've come to the conclusion that: 1. The old __name__ based behaviour was broken (since __name__ may not be the actual module name, as in "__main__" or when a pseudo-module is lying about its identity) 2. The 3.3 behaviour is still broken for similar reasons *and* added the dependency on __loader__ 3. Even PEP 451 still underspecifies the semantics of reloading (e.g. it's implied create_module isn't called for reload(), but not stated explicitly) 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. Side note: in "How Loading Will Work", _init_module_attrs needs to be passed both the module *and* the spec (current text only passes it the module). It also needs to be updated to handle the namespace package case (as shown below) And a sketch of some possible fully specified reload semantics: def reload(module): # Get the name to reload from the spec if it is available, otherwise __name__ 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 # We leave __name__ alone, so reloading __main__ has some hope of working correctly _init_module_attrs(loaded_module, updated_spec, set_name=False) # 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) else: loader.load_module(name_to_reload) # Reloading modules that replace themselves in sys.modules doesn't work reloaded_module = sys.modules[name_to_reload] if reloaded_module is not loaded_module: # Perhaps this should initially be a DeprecationWarning? # We've never actively *prevented* this case before, even though it is # almost guaranteed to break things if you do it. Alternatively, we could # leave "imp.reload()" as the more permissive version, and include this # check in the new importlib.reload() (with imp.reload calling an internal # importlib._unchecked_reload() instead) msg = "Module {!r} was replaced in sys.modules during reload" raise ImportError(msg.format(name_to_reload), name=name_to_reload) return reloaded_module -- 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