Update deepreload to use a rewritten knee.py. Fixes dreload(numpy).

[bfroehle]

knee.py, a Python re-implementation of hierarchical module import was
removed from the standard library because it no longer functioned properly.

deepreload.py is little more than a hacked version of knee.py which overrides
__builtin__.__import__ to ensure that each module is re-imported once (before
just referring to sys.modules as usual).

In addition, os.path was added to the default excluded modules, since
somehow it has an entry in sys.modules without `os' being a package.

I've posted my original re-written version of knee.py at https://gist.github.com/1944819. This would close gh-32.

[fperez]

@bfroehle, I'm really sorry for the long delay in reviewing this, but March was insane with travel and conferences. This is absolutely fantastic, thanks!!!

I'd only suggest that you also add a simple test that does import numpy; dreload(numpy), which would fail with current master and pass with your changes. That can be protected with a decorator so it only runs if people have numpy installed, we have such decorators already available, so you just have to do

from IPython.testing import decorators as dec
@dec.skipif_not_numpy
def test_deepreload():
  import numpy
  dreload(numpy)

[bfroehle]

Thanks for the unit test suggestion. The code provided worked, except that I needed to add 'unittest' to the list of excluded modules to prevent an error.

I suppose there should be a way to test that the deep reloading works, e.g.

def test_deepreload():
    import something_which_imports_string as m # What to choose here?
    x = string.Template('')

    dreload(m, exclude=['string'])
    nt.assert_is_instance(x, string.Template)

    dreload(m)
    nt.assert_not_is_instance(x, string.Template)

[fperez]

Yes, that would be possible, but I consider that to be above and beyond the expected call of duty here :) You'd have to create dynamically two modules in temporary files, modify the 2nd by rewriting the file, calling dreload() and then checking that the new values are correct. It's not really difficult, just some extra work. If you want to do that let me know and I'll wait, otherwise I think it's OK to proceed with merging.

[bfroehle]

The unit test need not be that complicated. As I alluded to above, we could check verify that the module was reloaded by seeing if an instance of a class of the original module is no longer an instance of the class of the reloaded module.

--- A.py ---
class Object(object):
    pass

--- B.py ---
import A

--- test_deepreload.py ---
import nose.tools as nt
def test_deepreload():
    import A
    import B # B imports A
    obj = A.Object()

    dreload(B, exclude=['A'])
    nt.assert_is_instance(obj, A.Object)

    dreload(B)
    nt.assert_not_is_instance(obj, A.Object)

However I'm not convinced that it's worth creating this elaborate framework (of several modules) for this one test. So I think we can just go ahead with the merge.

[fperez]

good point. Since you have it basically written already (and making those two files just requires this code:

import tempfile
tmp = tempfile.mkdtemp()
import sys
from os.path import join
sys.path.insert(0, tmp)
with open(join(tmp, 'A.py'), 'w') as f:
    f.write('class Object(object):\n    pass\n')
with open(join(tmp, 'B.py'), 'w') as f:
    f.write('import A')
try:
    # do rest of test here
    import A # etc...
finally:
    import shutil
    shutil.rmtree(tmp)

I guess we might as well put it in :) Do you want to go ahead and finish it? It will mean that this code at least has one real test for it, which does help.

[fperez]

ps - the above is a quick draft, the stdlib imports should go atop the main file, as usual

[bfroehle]

Okay, I think this is pretty much done. Do I need to cleanup the addition to sys.path? Other IPython tests are pretty inconsistent in this manner, some doing it, and others not.

[fperez]

Sorry, forgot to do that. Yes, it's probably a good idea to do undo the sys.path change. We should probably just have a context manager for that:

with temp_sys_path():
  sys.path.whatever()
  ...
# back to normal sys.path

but don't worry about it, just undo it with a sys.path.pop(0) in the finally clause.

[bfroehle]

Apparently such a context manager exists: IPython.utils.syspathcontext.prepended_to_syspath. Unfortunately we cannot use a more compact syntax since we need Python 2.6 compatibility:

with TemporaryDirectory() as tmpdir, prepended_to_syspath(tmpdir):
    ...

[fperez]

Looks great, thanks! Merging now. Thanks a ton for your patience despite my delayed reply...