Get help mid-command #480

merged 9 commits into from Jun 22, 2011

4 participants

IPython member

As pointed out in #98, it's common to be trying to use a method or object, and suddenly realise you want help on it. So it would be nice if:

abc = someobject.someotherobject.somemethod?

Gave you the help for the relevant method. These changes do that.

Also, when you do the above, the command will be placed back at the next prompt. This only applies when the object to be looked up isn't the only thing on the line, and is achieved with a new next_input parameter for ip.magic.


I have wanted this too many times to count. +1000 for the idea, but I haven't had time to review the code yet.

IPython member

Glad I'm not the only one who tries to do this!

I should have noted above - this PR also includes a couple of more or less unrelated things that happened while I was deciding how I wanted to make it work: some updates to the docstrings in inputsplitter, and an extra tool for the test suite.

IPython member

No, you aren't the only one - I think ~99% of users really want this. Great work!

I'll do a read through this afternoon, but this is not code I am familiar with.

@minrk minrk and 1 other commented on an outdated diff Jun 7, 2011
@@ -1649,7 +1649,8 @@ class InteractiveShell(SingletonConfigurable, Magic):
[D:\ipython]|1> _ip.set_next_input("Hello Word")
[D:\ipython]|2> Hello Word_ # cursor is here
+ if isinstance(s, unicode):
+ s = s.encode(self.stdin_encoding)
minrk Jun 7, 2011 IPython member

Haven't we been trying to use encode(enc, 'replace') for safety in general?

takluyver Jun 7, 2011 IPython member

Oh yes, thanks, Min. Updated.

@minrk minrk commented on the diff Jun 11, 2011
@@ -1787,7 +1788,7 @@ class InteractiveShell(SingletonConfigurable, Magic):
from . import history
- def magic(self,arg_s):
+ def magic(self, arg_s, next_input=None):
minrk Jun 11, 2011 IPython member

I am a bit concerned about changing the magic signature for a single method. Can you not call set_next_input inside the help call?

takluyver Jun 11, 2011 IPython member

I considered several ways to do this, and this was the one I preferred. The difficulty is that we only want to reproduce the line if it was like c = a.b? rather than a.b? or ?a.b. But it makes sense to abstract away that difference in the inputsplitter transforms, and just call magic('pinfo a.b.') in each case. So the execution layer doesn't know whether to call set_next_cell, and it seems wrong for the transformation layer to call it (and over ZMQ, it can't). Creating and reparsing an option flag for pinfo just seems awkward.

I also tried transforming it into two statements - the magic() call, then the set_next_input() call, separated by a semicolon. But that caused some trouble because the command became two AST nodes, so the input behaved differently. I switched to this system in 38c3c01.

This could also potentially be used by other magic commands, and it can't cause any issues with existing magic commands.

fperez Jun 22, 2011 IPython member

The idea of adding a parameter to a magic isn't really anathema, we've already done it in a few places, as you can see:

64 matches for "def magic_" in buffer:
    303:    def magic_lsmagic(self, parameter_s = ''):
    311:    def magic_magic(self, parameter_s = ''):
    416:    def magic_automagic(self, parameter_s = ''):
    443:    def magic_autocall(self, parameter_s = ''):
    510:    def magic_page(self, parameter_s=''):
    535:    def magic_profile(self, parameter_s=''):
    539:    def magic_pinfo(self, parameter_s='', namespaces=None):
    561:    def magic_pinfo2(self, parameter_s='', namespaces=None):
    569:    def magic_pdef(self, parameter_s='', namespaces=None):
    583:    def magic_pdoc(self, parameter_s='', namespaces=None):
    590:    def magic_psource(self, parameter_s='', namespaces=None):
    594:    def magic_pfile(self, parameter_s=''):
    617:    def magic_psearch(self, parameter_s=''):
    730:    def magic_who_ls(self, parameter_s=''):
    771:    def magic_who(self, parameter_s=''):
    832:    def magic_whos(self, parameter_s=''):
    967:    def magic_reset(self, parameter_s=''):
   1014:    def magic_reset_selective(self, parameter_s=''):
   1087:    def magic_xdel(self, parameter_s=''):
   1104:    def magic_logstart(self,parameter_s=''):
   1210:    def magic_logstop(self,parameter_s=''):
   1218:    def magic_logoff(self,parameter_s=''):
   1224:    def magic_logon(self,parameter_s=''):
   1234:    def magic_logstate(self,parameter_s=''):
   1239:    def magic_pdb(self, parameter_s=''):
   1273:    def magic_debug(self, parameter_s=''):
   1288:    def magic_prun(self, parameter_s ='',user_mode=1,
   1461:    def magic_run(self, parameter_s ='',runner=None,
   1755:    def magic_timeit(self, parameter_s =''):
   1893:    def magic_time(self,parameter_s = ''):
   1986:    def magic_macro(self,parameter_s = ''):
   2063:    def magic_save(self,parameter_s = ''):
   2104:    def magic_pastebin(self, parameter_s = ''):
   2115:    def magic_loadpy(self, arg_s):
   2264:    def magic_ed(self,parameter_s=''):
   2269:    def magic_edit(self,parameter_s='',last_call=['','']):
   2456:    def magic_xmode(self,parameter_s = ''):
   2475:    def magic_colors(self,parameter_s = ''):
   2545:    def magic_pprint(self, parameter_s=''):
   2556:    def magic_alias(self, parameter_s = ''):
   2628:    def magic_unalias(self, parameter_s = ''):
   2639:    def magic_rehashx(self, parameter_s = ''):
   2718:    def magic_pwd(self, parameter_s = ''):
   2731:    def magic_cd(self, parameter_s=''):
   2865:    def magic_env(self, parameter_s=''):
   2870:    def magic_pushd(self, parameter_s=''):
   2885:    def magic_popd(self, parameter_s=''):
   2894:    def magic_dirs(self, parameter_s=''):
   2899:    def magic_dhist(self, parameter_s=''):
   2936:    def magic_sc(self, parameter_s=''):
   3052:    def magic_sx(self, parameter_s=''):
   3091:    def magic_bookmark(self, parameter_s=''):
   3149:    def magic_pycat(self, parameter_s=''):
   3224:    def magic_quickref(self,arg):
   3231:    def magic_doctest_mode(self,parameter_s=''):
   3321:    def magic_gui(self, parameter_s=''):
   3347:    def magic_load_ext(self, module_str):
   3351:    def magic_unload_ext(self, module_str):
   3355:    def magic_reload_ext(self, module_str):
   3360:    def magic_install_profiles(self, s):
   3386:    def magic_install_default_config(self, s):
   3414:    def magic_pylab(self, s):
   3451:    def magic_tb(self, s):
   3458:    def magic_precision(self, s=''):

Since there's already precedent, and this is so incredibly useful (I've also wanted this a million times), I have no objection.

I think post 0.11 we'll want to give the magic system a long, hard look for a nice architectural cleanup, but for now this is a perfectly acceptable solution in my opinion. Practicality vs purity and all that :)

Go, great work!

IPython member

If @fperez is okay with adding an arg to the base magic method, then I think this is ready.

IPython member

ps - and don't forget to add mention of this in the what's new doc, at this point if it doesn't get put in we'll never have time to document it for the release :)

IPython member

Just to note - this is slightly different in that we're adding an argument to the magic() call itself, but the point about practicality still holds. I'll add it in what's new, then merge.

IPython member

Ah, right. Still, since eventually we'll want to cleanup the entire magic system, a tiny bit more cruft for such a useful payoff is not the end of the world as far as I'm concerned. Thanks for the good work, as always!

@takluyver takluyver merged commit b868e60 into ipython:master Jun 22, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment