Code review comment for lp:~ipython-dev/ipython/set_trace

On Mon, Jul 27, 2009 at 9:08 PM, Ondrej Certik<email address hidden> wrote:
>> If it simplifies embedding, then yes, I think we would consider it.  But, it
>> should go into Shell.py, which I have just refactored for the 0.11 release
>> (won't go into 0.10).  Are you OK waiting for the 0.11 for this - it will be
>> much cleaner to implement then.
>
> Sure, I can move it whenever is the best and sure I can wait for 0.11. Are you ok with me adding a stub into the __init__.py, so that this can be invoked just by:
>
> import IPython; IPython.embed()

Yes, I'm also OK with having a simplified embed() be part of the
top-level public API. But it should at least take a few arguments.
Basically you are 'folding' into a single embed() call the creation of
 an IPShellEmbed instance and its activation:

embed() <==> IPShellEmbed()()

But if we look at the signatures for IPShellEmbed's constructor and
activation, we see:

    def __init__(self,argv=None,banner='',exit_msg=None,rc_override=None,
                 user_ns=None):
    def __call__(self,header='',local_ns=None,global_ns=None,dummy=None):

so I'd suggest making the embed() signature something that reasonably
merges the two:

def embed(argv=None,banner='',exit_msg=None,rc_override=None,
     header='',local_ns=None,global_ns=None)

where the relevant arguments are fed through to either the __init__ or
the __call__ as appropriate.

This makes it consistent with the API it's wrapping for convenience
(only where it makes sense, so things like dummy which are meant for
the stateful part of the object aren't needed in this form).

With such caveats, I have no objections to exposing this at the
top-level. It is a very useful feature, so having the one-liner
being even easier to remember is a net win as far as I see it.

Thanks!

f