Merge lp://qastaging/~robert-kern/ipython/pretty into lp://qastaging/ipython/0.11

Check out the docstring of IPython/Extensions/ipy_pretty.py for instructions on how to use it. Give it a whirl, and let me know if anything goes wrong.

Some quick thoughts:

- Use ip.options.pprint instead of rc
- Raise TryNext if pprint is not on (instead of doing repr on your own)
- Move pretty.py to IPython.external
- Call set_hook("result_display", func, priority=99) since this is a 'catch-all' hook that should have a low priority (a hook that was interested in specific types of objects can override it). 100 is the priority of default hook

I didn't try it yet, but this popped to my mind quickly.

Done.

Fine with me

A few comments. This is great, but our extensions setup right now is a mess, see my email on list. Why don't you modify it to wrap the activation hook into a

def load():
   ....

method for now, and we can change the API later in trunk if we come to a different naming convention.

I'd also suggest putting your numpy prettyprinter as real code, instead of in-docstring, so that users can just load it themselves from your extension.

Finally, can you provide some doctests? While currently the main iptest script skips extensions (too many of them break dangerously if loaded), they can be manually tested via

maqroll[scratch]> cat /home/fperez/ipython/ipython/IPython/Extensions/ipy_foo.pydef foo():
    """simple doctest

    In [1]: print 1+2
    3

    In [2]: 1+2
    Out[2]: 3
    """
    return 1

maqroll[scratch]> iptest IPython.Extensions.ipy_foo
----------------------------------------------------------------------
Ran 1 test in 0.002s

OK

###

So a simple docstring with some examples that do the most basic showing and testing would be great. I'm trying to be good at not putting *any* new code that's fully tested.

If you add some test code, I'll manually whitelist it in iptest for now, until we get to the point where we can stop blacklisting all extensions.

Otherwise, fine with me. Thanks!

On Sat, Mar 14, 2009 at 19:34, Fernando Perez <email address hidden> wrote:
> Review: Needs Fixing
> A few comments.  This is great, but our extensions setup right now is a mess, see my email on list.  Why don't you modify it to wrap the activation hook into a
>
> def load():
>   ....
>
> method for now, and we can change the API later in trunk if we come to a different naming convention.

Done as activate(). We need a way to unset hooks in order to implement
deactivate().

> I'd also suggest putting your numpy prettyprinter as real code, instead of in-docstring, so that users can just load it themselves from your extension.

Done.

> Finally, can you provide some doctests?  While currently the main iptest script skips extensions (too many of them break dangerously if loaded), they can be manually tested via

I have not been able to get this to work. I get 0 tests with your
example. Anyways, the doctestable docstring is there.

--
Robert Kern

"I have come to believe that the whole world is an enigma, a harmless
enigma that is made terrible by our own mad attempt to interpret it as
though it had an underlying truth."
  -- Umberto Eco

On Sun, Mar 15, 2009 at 2:34 AM, Fernando Perez <email address hidden> wrote:
> Review: Needs Fixing

> A few comments.  This is great, but our extensions setup right now is a mess, see my email on list.  Why don't you modify it to wrap the activation hook into a
>
> def load():
>   ....
>
> method for now, and we can change the API later in trunk if we come to a different naming convention.

See _ip.load??

--
Ville M. Vainio
http://tinyurl.com/vainio

On Sat, Mar 14, 2009 at 6:24 PM, Robert Kern
<email address hidden> wrote:
> On Sat, Mar 14, 2009 at 19:34, Fernando Perez <email address hidden> wrote:
>> Review: Needs Fixing
>> A few comments.  This is great, but our extensions setup right now is a mess, see my email on list.  Why don't you modify it to wrap the activation hook into a
>>
>> def load():
>>   ....
>>
>> method for now, and we can change the API later in trunk if we come to a different naming convention.
>
> Done as activate(). We need a way to unset hooks in order to implement
> deactivate().
>
>> I'd also suggest putting your numpy prettyprinter as real code, instead of in-docstring, so that users can just load it themselves from your extension.
>
> Done.

Thanks!

>> Finally, can you provide some doctests?  While currently the main iptest script skips extensions (too many of them break dangerously if loaded), they can be manually tested via
>
> I have not been able to get this to work. I get 0 tests with your
> example. Anyways, the doctestable docstring is there.

Cool. You may need my trunk-dev for that to work though, there's a
lot of work on the testing machinery I've done there.

I have to run now, I'll return to this tomorrow...

In the meantime, if anyone has a chance to look at the other branches
pending review, that would be great :)

OK, go for it. Your doctests don't run yet even with my branch, but that's because I have a bug in handling multiline ipython inputs. Don't worry about that, let's merge this now, while I fix that issue in my branch. The tests will then get automatically picked up (I'll update with your fixes when you merge, and will make sure I pick them up correctly).

Thanks!

Robert: feel free to merge yourself if you want. I think only a *few* of us in the ~ip-dev team should be actually doing merges, to avoid possible problems with merge history. We should have a 'mergers' informally defined group and mentor people in for merge-into-trunk work gradually, but you have enough experience with all this game (more than me :), that I think it's fair to say that you're part of said informally defined group.

We should probably discuss on list this to have an explicit policy, on who handles the final merge steps, since it's the one place where there's a chance for conflicts or history problems to appear if many people try to do it simultaneously...

Anyway, that's a topic for the list. Merge away, making sure you have fully updated from trunk first.

"""
OK, go for it. Your doctests don't run yet even with my branch, but that's because I have a bug in handling multiline ipython inputs. Don't worry about that, let's merge this now, while I fix that issue in my branch.
"""

BTW, I just fixed that and added tests:

http://bazaar.launchpad.net/~fdo.perez/ipython/trunk-dev/revision/1177

So you could try to put your changes on top of my branch to run your tests.

Robert: I went ahead and merged it as-is, to reduce the risk of conflicts with my large one. Brian already reviewed that and it's almost ready to merge too.

Before 0.10, we can look into using _ip.load() as Ville suggested, to rationalize the extension loading API, but now at least the main code is in.

Thanks a lot for the work!