homepage

PJE brought up concerns on python-dev regarding PEP 451 and module reloading. [1] However, the issue isn't with the PEP changing reload semantics (mostly). Those actually changed with the switch to importlib (and a pure Python reload function) in the 3.3 release.
Nick sounded positive on fixing it, while Brett did not sound convinced it is worth it. I'm +1 as long as it isn't too complicated to fix. While we hash that out, here's a patch that hopefully demonstrates it isn't too complicated. :)
[1] https://mail.python.org/pipermail/python-dev/2013-October/129863.html 

It's actually even simpler than that - we can just go back to ignoring the __loader__ attribute entirely and always searching for a new one, since we want to pick up changes to the import hooks, even for modules with a __loader__ already set (which is almost all of them in 3.3+)
I'm not sure it's worth fixing in 3.3 though, as opposed to just formally specifying the semantics in PEP 451 (as noted on python-dev).

Here's an updated patch.

Patch mostly looks good to me, but there should be a second test ensuring that the loader attribute gets *replaced*, even if it is already set to something else.
A similar test structure to the existing one should work, but replacing the "del types.__loader__" with:
 expected_loader_type = type(types.__loader__)
 types.__loader__ = "this will be replaced by the reload"
and then later:
 assertIs(type(types.__loader__), expected_loader_type)

Brett: any opinions on fixing this? 3.3?
Nick: I'll add another test when I get a chance.

Just had a thought on a possible functional test case:
- write a module file
- load it
- check for expected attributes
- move it from name.py to name/__init__.py
- reload it
- check for new expected attributes

Fine with fixing it, but in context of PEP 451, not 3.3.

I'm fine with not fixing this for 3.3. Does this need to wait on PEP 451 acceptance?

Failing test case showing that Python 3.3 can't reload a module that is converted to a package behind importlib's back (I like this better than the purely introspection based tests, since it shows a real, albeit obscure, regression due to this behavioural change):
$ ./broken_reload.py 
E
======================================================================
ERROR: test_module_to_package (__main__.TestBadReload)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "./broken_reload.py", line 28, in test_module_to_package
 imp.reload(mod)
 File "/usr/lib64/python3.3/imp.py", line 271, in reload
 return module.__loader__.load_module(name)
 File "<frozen importlib._bootstrap>", line 586, in _check_name_wrapper
 File "<frozen importlib._bootstrap>", line 1024, in load_module
 File "<frozen importlib._bootstrap>", line 1005, in load_module
 File "<frozen importlib._bootstrap>", line 562, in module_for_loader_wrapper
 File "<frozen importlib._bootstrap>", line 855, in _load_module
 File "<frozen importlib._bootstrap>", line 950, in get_code
 File "<frozen importlib._bootstrap>", line 1043, in path_stats
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmp_n48mm/to_be_reloaded.py'
----------------------------------------------------------------------
Ran 1 test in 0.002s
FAILED (errors=1)
Interactive session showing that import.c didn't have this problem, since it reran the whole search (foo is just a toy module I had lying around in my play directory):
$ python
Python 2.7.5 (default, Oct 8 2013, 12:19:40) 
[GCC 4.8.1 20130603 (Red Hat 4.8.1-1)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import foo
Hello
>>> foo.__file__
'foo.py'
>>> import os
>>> os.mkdir("foo")
>>> os.rename('foo.py', 'foo/__init__.py')
>>> reload(foo)
Hello
<module 'foo' from 'foo/__init__.py'>
>>> foo.__file__
'foo/__init__.py'
>>>

No, the fix can go into Python 3.4 right now.

New changeset 88c3a1a3c2ff by Eric Snow in branch 'default':
Issue #19413: Restore pre-3.3 reload() semantics of re-finding modules.
http://hg.python.org/cpython/rev/88c3a1a3c2ff 

As you can see, Nick, I came up with a test that did just about the same thing (which you had suggested earlier :-) ). For good measure I also added a test that replaces a namespace package with a normal one.

Looks like this broke on windows:
======================================================================
FAIL: test_reload_namespace_changed (test.test_importlib.test_api.Source_ReloadTests)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "D:\cygwin\home\db3l\buildarea3円.x.bolen-windows\build\lib\test\test_importlib\test_api.py", line 283, in test_reload_namespace_changed
 [os.path.dirname(bad_path)] * 2)
AssertionError: Lists differ: ['C:\[46 chars]spam'] != ['C:\[46 chars]spam', 'C:\\DOCUME~1\\db3l\\LOCALS~1\\Temp\\tmpxhxk6rt9\\spam']
Second list contains 1 additional elements.
First extra element 1:
C:\DOCUME~1\db3l\LOCALS~1\Temp\tmpxhxk6rt9\spam
- ['C:\\DOCUME~1\\db3l\\LOCALS~1\\Temp\\tmpxhxk6rt9\\spam']
? ^
+ ['C:\\DOCUME~1\\db3l\\LOCALS~1\\Temp\\tmpxhxk6rt9\\spam',
? ^
+ 'C:\\DOCUME~1\\db3l\\LOCALS~1\\Temp\\tmpxhxk6rt9\\spam']

New changeset 78d36d54391c by Eric Snow in branch 'default':
Issue #19413: Disregard duplicate namespace portions during reload tests.
http://hg.python.org/cpython/rev/78d36d54391c 

Windows looks happy now. I'll look into the duplicate portions separately in issue19469.