[IPython-dev] Outstanding pull requests

Sun Feb 13 20:01:31 EST 2011

Hi Thomas,

I just wanted to thank you *enormously* for doing this.  I have to
switch gears out of ipython right now for some deadlines, but later in
the week I'll return to this, and I'll try to flush a few of the easy
ones out of the way.  Some comments below.

On Sun, Feb 13, 2011 at 9:45 AM, Thomas Kluyver <takowl at gmail.com> wrote:
> By way of my debut in the core team, I've prepared a list of all the open
> pull requests, with a very brief assessment of each, in the hope that we can
> start sorting them out (we're currently at 31 open requests):
>
> Magic functions documentation (several branches from SciPy India):
> [The first two listed here seem to have changed various bits of indentation
> beyond the specific things they're described as changing. This makes the
> diffs rather long. Do we want to copy the key changes out in new commits,
> using the --author option to credit them?]

Good idea.  These docstring requests were done by a number of teams at
scipy india who were at the very beginning of their python/programming
experience, so they are more likely to have beginner's problems.

It's a real bummer that I dropped the ball in providing them with
early feedback, but I had a ton of travel right after and just got
overrun with life...

So definitely crediting with --author but cleaning up the commit is a
good strategy for those with unnecessary noise.

> - cd: looks like some examples are at the bash prompt, not the ipython
> prompt. https://github.com/ipython/ipython/pull/233/files
> - profile: Has neatly documented the Python profile module. Unfortunately,
> IPython's %profile command does something entirely different.
> https://github.com/ipython/ipython/pull/232/files
> - logstart: Seems OK, although there's an "examples" title added for logstop
> with no actual examples. logstop probably doesn't need an example, but we
> should tidy up the "examples" title.
> https://github.com/ipython/ipython/pull/231/files
> - edit: Clean diff, but giving an example for edit is tricky, because you
> can't show the interactive editor. Also restates the -p option in a comment
> in the example. https://github.com/ipython/ipython/pull/230/files (closed
> 220 as duplicate)
> - pdef (it's confusing to use magic functions as the arguments for the
> example, we should have a user defined function) & colors (one example is
> probably enough, and the last two are wrong, using %usage instead of
> %colors) https://github.com/ipython/ipython/pull/229/files
> - who, whos, who_ls: Looks good. https://github.com/ipython/ipython/pull/228
> - logon: Not quite sure what's happened here. Example seems to be a bunch of
> Java-ish code. It probably needs to be redone from scratch.
> https://github.com/ipython/ipython/pull/227
> - pwd: pwd example is good, but has also added to exit (doesn't really need
> an example), dirs (example doesn't really show what it's for), and color
> (just an "examples" heading, no actual examples.
> https://github.com/ipython/ipython/pull/226/files
> - save: Clean diff, but the example and explanation aren't very clear.
> Probably needs to be redone from scratch.
> https://github.com/ipython/ipython/pull/225
> - cpaste: Appears to have ripped out the actual function while writing the
> docs, and stitched its definition onto the magic_paste function code.
> Example could also be a bit clearer.
> https://github.com/ipython/ipython/pull/224
> - colors: Same changes (& problems) as request 229.
> https://github.com/ipython/ipython/pull/223
> - timeit: Too many equivalent examples.
> https://github.com/ipython/ipython/pull/222/files
> - alias: Already had examples, changes remove one of them. Adds two, but one
> (ls) is available without aliasing, and the other is inaccurate (the example
> shows the result of a man command).
> https://github.com/ipython/ipython/pull/221
> - pycat: Doesn't actually use a Python file, so rather defeats the point of
> pycat. Not sure that an example is really helpful, as it doesn't show the
> colouring you get at a terminal. https://github.com/ipython/ipython/pull/219
> - hist: Formatting is off (probably related to a bug in the code). Also, the
> "this is an example of..." line seems redundant.
> https://github.com/ipython/ipython/pull/218/files
>
> Whew. Of these, I recommend merging 228 (%who), and closing 225 (%save), 232
> (%profile) with "try again" and 227 (%logon), for which I'm not sure that
> it's even possible to do an example.
>
> The others will need further work. Fernando, any sort of feel for how many
> of the SciPy India contributors will still be in the loop to revise their
> contributions?

Thanks for going through all of this!  Unfortunately due to the delay,
and the fact that many of them were first-timers to pretty much
everything, I'm not too hopeful.  I think if there are some where we
can easily finish up what they did with minimal work, it would be a
nice gesture.  But those that require significant rework, just
dropping a comment would be sufficient.  If they do pop up again and
come back with improvements, then we can engage them better for the
future.

In my defense, I was trying to handle a sprint where 60! people wanted
to contribute, most of which had never coded a single line of Python.
So I tried to do something where at least they would learn during the
day, even if it wasn't realistic to expect actual final contributions
to the codebase to come from everyone.

> Looking at the others:
>
> Small pull requests:
> - %hist output format: I've solved the same problem in a different way in
> pull request 261, see below. https://github.com/ipython/ipython/pull/236
> - Logging unicode characters (0.10 series): Good start. See my comment there
> for minor concern. https://github.com/ipython/ipython/pull/249
> - check_gtk: Removes a function. Mark Voorhies has tested the change and
> commented that it works. I'll admit to not understanding what it's about.
> https://github.com/ipython/ipython/pull/237
> - Typos in zmq/blockingkernelmanager.py: Seems OK to me.
> https://github.com/ipython/ipython/pull/203/files
> - cProfiling ipython.py: Updated in response to Brian's concerns. Needs
> reviewing again. https://github.com/ipython/ipython/pull/193
> - Autocomplete in Emacs: Removes the all_completions function (which
> apparently was broken). One other person says it works. Needs review.
> https://github.com/ipython/ipython/pull/204
> - Unicode issues [My changes]: Tests now integrated into main test suite
> (thanks to help from Robert Kern). Needs review.
> https://github.com/ipython/ipython/pull/252
> - Wildcard module [My changes]: Fixes issue 129. The bug and the fix are
> pretty much self contained, so shouldn't interfere with changes in other
> code. No feedback yet. (this is pushing what I'd call small, but shouldn't
> change functionality besides fixing the bug)
> https://github.com/ipython/ipython/pull/251
>
> Medium pull requests:
> - Fixing up various magic commands with the new history system [My changes]:
> Revised after Fernando's comments, unit tests added. Conflicts with
> Madhusudan's #203, see below. https://github.com/ipython/ipython/pull/261
> - %hist command bugs: This solves one of the same problems I've fixed in
> #261, in a different way. https://github.com/ipython/ipython/pull/203/files
> - Unbundling external libraries: Revised in response to Fernando's and
> Brian's concerns. Needs reviewing again.
> https://github.com/ipython/ipython/pull/191
> - Pyside compatibility: Waiting for contributor to resolve bug with SVG
> handling. https://github.com/ipython/ipython/pull/259
> - Kernel logging: Fernando has requested a final check from Robert.
> https://github.com/ipython/ipython/pull/264
>
> Larger pull requests:
> - Magic arguments decorators. Updated in response to Fernando's comments.
> Needs reviewing again. https://github.com/ipython/ipython/pull/199
>
> Not quite yet:
> - HTML Notebook: Gained a lot of interest, but not quite ready yet. Possible
> project for GSoC this year. (But if we're going to accumulate alternative
> frontends, should we consider putting each frontend in its own repo?)
> https://github.com/ipython/ipython/pull/179
> - MinRK's newparallel: Work in progress. Lots of
> work.https://github.com/ipython/ipython/pull/254

Ok, thanks again for this analysis.

Over the next few days, I suggest that anyone willing to 'take
ownership' of one just pitch in and add a comment to the corresponding
one, mentioning they'll keep an eye on it.  That way, we can split
this load and claw our way back to a small number of pending ones.

Best,

f