[PATCH 2 of 2] demandimport: omit default value of "level" at construction of _demandmod

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Sun Aug 7 14:48:53 EDT 2016


At Sun, 7 Aug 2016 18:18:17 +0200,
Pierre-Yves David wrote:
> 
> On 08/07/2016 05:42 PM, FUJIWARA Katsunori wrote:
> > At Mon, 8 Aug 2016 00:20:35 +0900,
> > Yuya Nishihara wrote:
> >>
> >> On Sun, 07 Aug 2016 23:59:37 +0900, FUJIWARA Katsunori wrote:
> >>>>>              if symbol is nothing:
> >>>>>                  mn = '%s.%s' % (mod.__name__, attr)
> >>>>>                  if mn in ignore:
> >>>>> -                    importfunc = _origimport
> >>>>> +                    symbol = _origimport(attr, mod.__dict__, locals, level=1)
> >>>>>                  else:
> >>>>> -                    importfunc = _demandmod
> >>>>> -                symbol = importfunc(attr, mod.__dict__, locals, level=1)
> >>>>> +                    symbol = _demandmod(attr, mod.__dict__, locals, 1)
> >>>>
> >>>> This change is okay, but unrelated?
> >>>>
> >>>
> >>> _demandmod.__init__() requires no optional argument before "level",
> >>> but __import__() as originalimport() requires "fromlist" as below:
> >>>
> >>>     __import__(name, globals={}, locals={}, fromlist=[], level=-1)
> >>>
> >>> Therefore, just passing "1" as level argument instead of "level=1"
> >>> invokes original __import__() with "fromlist=1" :-<
> >>>
> >>>     @@ -196,7 +196,7 @@ def _demandimport(name, globals=None, lo
> >>>                          importfunc = _origimport
> >>>                      else:
> >>>                          importfunc = _demandmod
> >>>     -                symbol = importfunc(attr, mod.__dict__, locals, level=1)
> >>>     +                symbol = importfunc(attr, mod.__dict__, locals, 1)
> >>>                      setattr(mod, attr, symbol)
> >>
> >> Both functions can take level=1 as before. I don't strongly disagree with
> >> this change, but IMHO, 'level=1' is clearer than bare '1'. (I would write
> >> '/*level=*/ 1' in C.)
> >>
> >
> > Ah, I focused on invoking _demandmod.__init__() with explicit "1" like
> > as previous patch in this series, but keeping "level=" is better for
> > readability as you described.
> >
> > Should I post follow up patch to rework this chunk ?
> 
> There is very few changeset over it, if you tell me how to amend it, I 
> would be happy to do so.

For patch #1, please add "level=" for bare level value 1 as below:

================
diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -117,7 +117,8 @@ class _demandmod(object):
                 if '.' in p:
                     h, t = p.split('.', 1)
                 if getattr(mod, h, nothing) is nothing:
-                    setattr(mod, h, _demandmod(p, mod.__dict__, mod.__dict__))
+                    setattr(mod, h,
+                            _demandmod(p, mod.__dict__, mod.__dict__, level=1))
                 elif t:
                     subload(getattr(mod, h), t)
 
@@ -211,7 +212,8 @@ def _demandimport(name, globals=None, lo
             for comp in modname.split('.')[1:]:
                 if getattr(mod, comp, nothing) is nothing:
                     setattr(mod, comp,
-                            _demandmod(comp, mod.__dict__, mod.__dict__))
+                            _demandmod(comp, mod.__dict__, mod.__dict__,
+                                       level=1))
                 mod = getattr(mod, comp)
             return mod
 
================

For patch #2, please drop whole the 2nd diff hunk for demandimport.py
in this patch below:

================
@@ -194,10 +198,9 @@ def _demandimport(name, globals=None, lo
             if symbol is nothing:
                 mn = '%s.%s' % (mod.__name__, attr)
                 if mn in ignore:
-                    importfunc = _origimport
+                    symbol = _origimport(attr, mod.__dict__, locals, level=1)
                 else:
-                    importfunc = _demandmod
-                symbol = importfunc(attr, mod.__dict__, locals, level=1)
+                    symbol = _demandmod(attr, mod.__dict__, locals, 1)
                 setattr(mod, attr, symbol)
 
             # Record the importing module references this symbol so we can
================


> 
> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list