[PATCH] demandimport: store level argument on _demandmod instances
Renato Cunha
renatoc at gmail.com
Tue Aug 17 12:43:59 CDT 2010
Hello Dan,
On Tue, Aug 17, 2010 at 12:48, Dan Villiom Podlaski Christiansen
<danchr at gmail.com> wrote:
> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
> # Date 1282059970 -7200
> # Node ID 837e75faec6467cd2d6c51354d9ee02855af784f
> # Parent 98149ae642259e899e4440738bedac51f723f5ae
> demandimport: store level argument on _demandmod instances
>
> The 'level' argument to __import__ was added in Python 2.6, and is
> specified for either relative or absolute imports. The fix introduced
> in e160f2312815 allowed such imports to proceed without failure, but
> effectively disabled demandimport for them. This is particularly
> unfortunate in Python 3.x, where *all* imports are either relative or
> absolute.
Or, from __import__'s docstring:
__import__(name, globals={}, locals={}, fromlist=[], level=-1) -> module
"Level is used to determine whether to perform absolute or relative imports.
-1 is the original strategy of attempting both absolute and relative imports, 0
is absolute, a positive number is the number of parent directories to search
relative to the current module."
But, in my tests, I've never seen level = -1. I'd assume that, for all
practical cases, we'll only get a non-negative level (python hackers, feel free
to enlighten us).
> The solution introduced here is to store the level argument on the
> demandimport instance, and propagate it to _origimport() when its
> value isn't None.
Maybe not everyone is aware of it, so maybe it might be nice to state that
__import__ in python < 2.6, __import__ takes four arguments:
>>> # in python 2.4
>>> __import__('sys', globals(), locals(), [], None)
Traceback (most recent call last):
File "<stdin>", line 1, in ?
TypeError: __import__() takes at most 4 arguments (5 given)
> Please note that this patch hasn't been tested in Python 3.x, and thus
> may not be complete.
I tested it manually in the interpreter and by running hg manifest in py3k and
it seems to be working. If it wasn't, the import cycles should've bitten me.
Also, running hg without demandimport runs:
* In 484ms (min) in my desktop with demandimport disabled;
* In 146ms (min) in my desktop with demandimport enabled and your patch on;
* In 142ms (min) in my desktop with demandimport enabled and your patch off;
And, on py3k, it runs:
* In 131ms (min) in my desktop with demandimport enabled and your patch on;
So we may assume that your patch does indeed work. (Additionally, the running
time of the test suite doesn't seem to have changed because of the patch.)
> I'm worried about how sub-imports are handled; I don't know what they are,
Take a look at the place where subload is called:
for x in after:
subload(mod, x)
Good, and how after is defined?
if '.' in name:
head, rest = name.split('.', 1)
after = [rest]
Considering the example "import mercurial.util", head = "mercurial" and
after = ["util"].
And that's your sub-import. (Am I right in assuming that after will always
either have length 0 or 1?)
> or whether the level argument should be modified for them. I've added 'TODO'
> notes to these cases; hopefully, someone more knowledgable of these issues
> will deal with them.
I don't think so. IMHO, the python interpreter will have already resolved the
level before passing it to __import__, so (I assume) there is no need to change
the level ("a positive number is the number of parent directories to search
relative to the current module").
Regarding the level, this is what I get from running the following experiment
(in py3k):
Suppose I have the tree:
a/c
a/c/d.py
a/c/__init__.py
a/__init__.py
a/b.py
And that the b module imports the d module in package c.
>>> import builtins
>>> origimport = builtins.__import__
>>> def i(name, globals, locals, fromlist, level):
... print('Importing name:', name)
... print('Fromlist:', fromlist)
... print('Level:', level)
... print('----')
... try:
... origimport(name, globals, locals, fromlist, level)
... except:
... pass
...
>>> builtins.__import__ = i
>>> import a
Importing name: a
Fromlist: None
Level: 0
----
>>> import a.b
Importing name: a.b
Fromlist: None
Level: 0
----
Importing name: c
Fromlist: ('d',)
Level: 1
----
> diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> --- a/mercurial/demandimport.py
> +++ b/mercurial/demandimport.py
> @@ -29,29 +29,35 @@ _origimport = __import__
>
> class _demandmod(object):
> """module demand-loader and proxy"""
> - def __init__(self, name, globals, locals):
> + def __init__(self, name, globals, locals, level):
> if '.' in name:
> head, rest = name.split('.', 1)
> after = [rest]
> else:
> head = name
> after = []
> - object.__setattr__(self, "_data", (head, globals, locals, after))
> + object.__setattr__(self, "_data", (head, globals, locals, after, level))
> object.__setattr__(self, "_module", None)
> def _extend(self, name):
> """add to the list of submodules to load"""
> self._data[3].append(name)
> def _load(self):
> if not self._module:
> - head, globals, locals, after = self._data
> - mod = _origimport(head, globals, locals)
> + head, globals, locals, after, level = self._data
> + if level is not None:
> + mod = _origimport(head, globals, locals, level)
> + else:
> + mod = _origimport(head, globals, locals)
> # load submodules
> def subload(mod, p):
> h, t = p, None
> if '.' in p:
> h, t = p.split('.', 1)
> if not hasattr(mod, h):
> - setattr(mod, h, _demandmod(p, mod.__dict__, mod.__dict__))
> + # TODO: should we adjust the level here?
> + submod = _demandmod(p, mod.__dict__, mod.__dict__,
> + level=level)
> + setattr(mod, h, submod)
> elif t:
> subload(getattr(mod, h), t)
>
> @@ -91,28 +97,36 @@ def _demandimport(name, globals=None, lo
> base, rest = name.split('.', 1)
> # email.__init__ loading email.mime
> if globals and globals.get('__name__', None) == base:
> - return _origimport(name, globals, locals, fromlist)
> + if level is not None:
> + return _origimport(name, globals, locals, fromlist, level)
> + else:
> + return _origimport(name, globals, locals, fromlist)
> # if a is already demand-loaded, add b to its submodule list
> if base in locals:
> if isinstance(locals[base], _demandmod):
> locals[base]._extend(rest)
> return locals[base]
> - return _demandmod(name, globals, locals)
> + return _demandmod(name, globals, locals, level=level)
> else:
> + # from a import b,c,d
> if level is not None:
> - # from . import b,c,d or from .a import b,c,d
> - return _origimport(name, globals, locals, fromlist, level)
> - # from a import b,c,d
> - mod = _origimport(name, globals, locals)
> + mod = _origimport(name, globals, locals, level=level)
> + else:
> + mod = _origimport(name, globals, locals)
> # recurse down the module chain
> for comp in name.split('.')[1:]:
> if not hasattr(mod, comp):
> - setattr(mod, comp, _demandmod(comp, mod.__dict__, mod.__dict__))
> + # TODO: should we adjust the level here?
> + submod = _demandmod(comp, mod.__dict__, mod.__dict__,
> + level=level)
> + setattr(mod, comp, submod)
> mod = getattr(mod, comp)
> for x in fromlist:
> # set requested submodules for demand load
> if not(hasattr(mod, x)):
> - setattr(mod, x, _demandmod(x, mod.__dict__, locals))
> + # TODO: should we adjust the level here?
> + submod = _demandmod(x, mod.__dict__, locals, level=level)
> + setattr(mod, x, submod)
> return mod
>
> ignore = [
>
Thus, I'm fine with the patch (but I'm no import expert).
Regards,
--
Renato Cunha <http://renatocunha.com>
Blog: http://valedotrovao.com
"Whatever happens, happens"
More information about the Mercurial-devel
mailing list