Patchwork [1,of,3] demandimport: define an importnow context manager

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date May 26, 2015, 8:35 p.m.
Message ID <ac165f67eab4ab8157f7.1432672500@Iris>
Download mbox | patch
Permalink /patch/9288/
State Changes Requested
Headers show

Comments

Jordi Gutiérrez Hermoso - May 26, 2015, 8:35 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1432671771 14400
#      Tue May 26 16:22:51 2015 -0400
# Node ID ac165f67eab4ab8157f73ab229e80883d49fabe0
# Parent  6ac860f700b5cfeda232d5305963047696b869ca
demandimport: define an importnow context manager

This can be useful for use in "with" blocks for temporarily disabling
demandimport.
Pierre-Yves David - May 26, 2015, 8:45 p.m.
On 05/26/2015 01:35 PM, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1432671771 14400
> #      Tue May 26 16:22:51 2015 -0400
> # Node ID ac165f67eab4ab8157f73ab229e80883d49fabe0
> # Parent  6ac860f700b5cfeda232d5305963047696b869ca
> demandimport: define an importnow context manager
>
> This can be useful for use in "with" blocks for temporarily disabling
> demandimport.
>
> diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> --- a/mercurial/demandimport.py
> +++ b/mercurial/demandimport.py
> @@ -25,6 +25,8 @@ These imports will not be delayed:
>   '''
>
>   import __builtin__, os, sys
> +from contextlib import contextmanager
> +
>   _origimport = __import__
>
>   nothing = object()
> @@ -179,3 +181,14 @@ def enable():
>   def disable():
>       "disable global demand-loading of modules"
>       __builtin__.__import__ = _origimport
> +
> +@contextmanager
> +def importnow():

what about: nodemandimport?

> +    demandenabled = isenabled()
> +    if demandenabled:
> +        disable()
> +
> +    yield
> +
> +    if demandenabled:
> +        enable()

You want:

    try:
        yield
    finally:
        if demandenabled:
             enable()


otherwise you will fail to restore in case of error.
Augie Fackler - May 27, 2015, 2:51 p.m.
On Tue, May 26, 2015 at 01:45:12PM -0700, Pierre-Yves David wrote:
>
>
> On 05/26/2015 01:35 PM, Jordi Gutiérrez Hermoso wrote:
> ># HG changeset patch
> ># User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> ># Date 1432671771 14400
> >#      Tue May 26 16:22:51 2015 -0400
> ># Node ID ac165f67eab4ab8157f73ab229e80883d49fabe0
> ># Parent  6ac860f700b5cfeda232d5305963047696b869ca
> >demandimport: define an importnow context manager
> >
> >This can be useful for use in "with" blocks for temporarily disabling
> >demandimport.
> >
> >diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> >--- a/mercurial/demandimport.py
> >+++ b/mercurial/demandimport.py
> >@@ -25,6 +25,8 @@ These imports will not be delayed:
> >  '''
> >
> >  import __builtin__, os, sys
> >+from contextlib import contextmanager
> >+
> >  _origimport = __import__
> >
> >  nothing = object()
> >@@ -179,3 +181,14 @@ def enable():
> >  def disable():
> >      "disable global demand-loading of modules"
> >      __builtin__.__import__ = _origimport
> >+
> >+@contextmanager
> >+def importnow():
>
> what about: nodemandimport?

Since we're painting this shed, what about immediate? So it'd be:

with demandimport.immediate():
  ...

(I really don't feel strongly about the name, Jordi, feel encouraged
to pick what seems sensible.)

>
> >+    demandenabled = isenabled()
> >+    if demandenabled:
> >+        disable()
> >+
> >+    yield
> >+
> >+    if demandenabled:
> >+        enable()
>
> You want:
>
>    try:
>        yield
>    finally:
>        if demandenabled:
>             enable()
>
>
> otherwise you will fail to restore in case of error.

+1. I'll await a resend once you two work out the name and will look
to see this change in the next round. Thanks!

>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Jordi Gutiérrez Hermoso - May 27, 2015, 6:39 p.m.
On Tue, 2015-05-26 at 13:45 -0700, Pierre-Yves David wrote:
> 
> On 05/26/2015 01:35 PM, Jordi Gutiérrez Hermoso wrote:
> > # HG changeset patch
> > # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> > # Date 1432671771 14400
> > #      Tue May 26 16:22:51 2015 -0400
> > # Node ID ac165f67eab4ab8157f73ab229e80883d49fabe0
> > # Parent  6ac860f700b5cfeda232d5305963047696b869ca
> > demandimport: define an importnow context manager
> >
> > This can be useful for use in "with" blocks for temporarily disabling
> > demandimport.
> >
> > diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> > --- a/mercurial/demandimport.py
> > +++ b/mercurial/demandimport.py
> > @@ -25,6 +25,8 @@ These imports will not be delayed:
> >   '''
> >
> >   import __builtin__, os, sys
> > +from contextlib import contextmanager
> > +
> >   _origimport = __import__
> >
> >   nothing = object()
> > @@ -179,3 +181,14 @@ def enable():
> >   def disable():
> >       "disable global demand-loading of modules"
> >       __builtin__.__import__ = _origimport
> > +
> > +@contextmanager
> > +def importnow():
> 
> what about: nodemandimport?

My only concern with that is that then it would read

    with demandimport.nodemandimport():

which akward to pash in English. If I'm going to be doing a negative,
I really wish I could write,

    without demandimport():

but of course this isn't valid Python.

I think I prefer Augie's recommendation to do

   with demandimport.immediate():
Pierre-Yves David - May 27, 2015, 6:42 p.m.
On 05/27/2015 11:39 AM, Jordi Gutiérrez Hermoso wrote:
> On Tue, 2015-05-26 at 13:45 -0700, Pierre-Yves David wrote:
>>
>> On 05/26/2015 01:35 PM, Jordi Gutiérrez Hermoso wrote:
>>> # HG changeset patch
>>> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
>>> # Date 1432671771 14400
>>> #      Tue May 26 16:22:51 2015 -0400
>>> # Node ID ac165f67eab4ab8157f73ab229e80883d49fabe0
>>> # Parent  6ac860f700b5cfeda232d5305963047696b869ca
>>> demandimport: define an importnow context manager
>>>
>>> This can be useful for use in "with" blocks for temporarily disabling
>>> demandimport.
>>>
>>> diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
>>> --- a/mercurial/demandimport.py
>>> +++ b/mercurial/demandimport.py
>>> @@ -25,6 +25,8 @@ These imports will not be delayed:
>>>    '''
>>>
>>>    import __builtin__, os, sys
>>> +from contextlib import contextmanager
>>> +
>>>    _origimport = __import__
>>>
>>>    nothing = object()
>>> @@ -179,3 +181,14 @@ def enable():
>>>    def disable():
>>>        "disable global demand-loading of modules"
>>>        __builtin__.__import__ = _origimport
>>> +
>>> +@contextmanager
>>> +def importnow():
>>
>> what about: nodemandimport?
>
> My only concern with that is that then it would read
>
>      with demandimport.nodemandimport():
>
> which akward to pash in English. If I'm going to be doing a negative,
> I really wish I could write,
>
>      without demandimport():
>
> but of course this isn't valid Python.
>
> I think I prefer Augie's recommendation to do
>
>     with demandimport.immediate():

Some black magic idea:

with demandimport.disable():

disable() both disable debug and return a context manager object able to 
restore previous value on __exit__
Jordi Gutiérrez Hermoso - May 28, 2015, 2:32 p.m.
On Wed, 2015-05-27 at 11:42 -0700, Pierre-Yves David wrote:
> Some black magic idea:
> 
> with demandimport.disable():
> 
> disable() both disable debug and return a context manager object
> able to restore previous value on __exit__

That's a bit too magical for my taste. But it gives me an idea. This
looks great:

    with demandimport.disabled():

I'll do that instead.
Augie Fackler - May 28, 2015, 3:11 p.m.
On Thu, May 28, 2015 at 10:32 AM, Jordi Gutiérrez Hermoso
<jordigh@octave.org> wrote:
> On Wed, 2015-05-27 at 11:42 -0700, Pierre-Yves David wrote:
>> Some black magic idea:
>>
>> with demandimport.disable():
>>
>> disable() both disable debug and return a context manager object
>> able to restore previous value on __exit__
>
> That's a bit too magical for my taste. But it gives me an idea. This
> looks great:
>
>     with demandimport.disabled():
>
> I'll do that instead.


FYI, this shed color is fine for this native speaker.
Pierre-Yves David - May 28, 2015, 4:30 p.m.
On 05/28/2015 07:32 AM, Jordi Gutiérrez Hermoso wrote:
> On Wed, 2015-05-27 at 11:42 -0700, Pierre-Yves David wrote:
>> Some black magic idea:
>>
>> with demandimport.disable():
>>
>> disable() both disable debug and return a context manager object
>> able to restore previous value on __exit__
>
> That's a bit too magical for my taste. But it gives me an idea. This
> looks great:
>
>      with demandimport.disabled():
>
> I'll do that instead.

one letter difference for similar intend but different usage function. 
what would possibily goes wrong…

Patch

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -25,6 +25,8 @@  These imports will not be delayed:
 '''
 
 import __builtin__, os, sys
+from contextlib import contextmanager
+
 _origimport = __import__
 
 nothing = object()
@@ -179,3 +181,14 @@  def enable():
 def disable():
     "disable global demand-loading of modules"
     __builtin__.__import__ = _origimport
+
+@contextmanager
+def importnow():
+    demandenabled = isenabled()
+    if demandenabled:
+        disable()
+
+    yield
+
+    if demandenabled:
+        enable()