Thanks for all this great work. I'll pitch in and merge some of these.<div><br></div><div>I just have a clarifying question for direct-commit/pull-request policy.</div><div><br></div><div>For instance, I was looking at %who, and the docstrings look great, but I noticed that the behavior in the code is actually wrong:</div>
<div><br></div><div> out = [i for i in out if type(i).__name__ in typeset]</div><div><br></div><div>should be:</div><div><br></div><div><div> out = [i for i in out if type(user_ns[i]).__name__ in typeset]</div>
<div><br></div><div>(since `out` is the list of <i>names</i>, not variables, so it behaved as if everything had type 'str'). Which, first of all, is excellent, since docstrings should help us find bugs like this one has, but here's my question:</div>
<div><br></div><div>What is the policy on tiny bugs like this? Should I do a one-line pull-request, or just push a fix directly to master (with test, of course)?</div><div><br></div><div>-MinRK</div><br><div class="gmail_quote">
On Sun, Feb 13, 2011 at 17:01, Fernando Perez <span dir="ltr"><<a href="http://fperez.net">fperez.net</a>@<a href="http://gmail.com">gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi Thomas,<br>
<br>
I just wanted to thank you *enormously* for doing this. I have to<br>
switch gears out of ipython right now for some deadlines, but later in<br>
the week I'll return to this, and I'll try to flush a few of the easy<br>
ones out of the way. Some comments below.<br>
<div class="im"><br>
On Sun, Feb 13, 2011 at 9:45 AM, Thomas Kluyver <<a href="mailto:takowl@gmail.com">takowl@gmail.com</a>> wrote:<br>
> By way of my debut in the core team, I've prepared a list of all the open<br>
> pull requests, with a very brief assessment of each, in the hope that we can<br>
> start sorting them out (we're currently at 31 open requests):<br>
><br>
> Magic functions documentation (several branches from SciPy India):<br>
> [The first two listed here seem to have changed various bits of indentation<br>
> beyond the specific things they're described as changing. This makes the<br>
> diffs rather long. Do we want to copy the key changes out in new commits,<br>
> using the --author option to credit them?]<br>
<br>
</div>Good idea. These docstring requests were done by a number of teams at<br>
scipy india who were at the very beginning of their python/programming<br>
experience, so they are more likely to have beginner's problems.<br>
<br>
It's a real bummer that I dropped the ball in providing them with<br>
early feedback, but I had a ton of travel right after and just got<br>
overrun with life...<br>
<br>
So definitely crediting with --author but cleaning up the commit is a<br>
good strategy for those with unnecessary noise.<br>
<div><div></div><div class="h5"><br>
> - cd: looks like some examples are at the bash prompt, not the ipython<br>
> prompt. <a href="https://github.com/ipython/ipython/pull/233/files" target="_blank">https://github.com/ipython/ipython/pull/233/files</a><br>
> - profile: Has neatly documented the Python profile module. Unfortunately,<br>
> IPython's %profile command does something entirely different.<br>
> <a href="https://github.com/ipython/ipython/pull/232/files" target="_blank">https://github.com/ipython/ipython/pull/232/files</a><br>
> - logstart: Seems OK, although there's an "examples" title added for logstop<br>
> with no actual examples. logstop probably doesn't need an example, but we<br>
> should tidy up the "examples" title.<br>
> <a href="https://github.com/ipython/ipython/pull/231/files" target="_blank">https://github.com/ipython/ipython/pull/231/files</a><br>
> - edit: Clean diff, but giving an example for edit is tricky, because you<br>
> can't show the interactive editor. Also restates the -p option in a comment<br>
> in the example. <a href="https://github.com/ipython/ipython/pull/230/files" target="_blank">https://github.com/ipython/ipython/pull/230/files</a> (closed<br>
> 220 as duplicate)<br>
> - pdef (it's confusing to use magic functions as the arguments for the<br>
> example, we should have a user defined function) & colors (one example is<br>
> probably enough, and the last two are wrong, using %usage instead of<br>
> %colors) <a href="https://github.com/ipython/ipython/pull/229/files" target="_blank">https://github.com/ipython/ipython/pull/229/files</a><br>
> - who, whos, who_ls: Looks good. <a href="https://github.com/ipython/ipython/pull/228" target="_blank">https://github.com/ipython/ipython/pull/228</a><br>
> - logon: Not quite sure what's happened here. Example seems to be a bunch of<br>
> Java-ish code. It probably needs to be redone from scratch.<br>
> <a href="https://github.com/ipython/ipython/pull/227" target="_blank">https://github.com/ipython/ipython/pull/227</a><br>
> - pwd: pwd example is good, but has also added to exit (doesn't really need<br>
> an example), dirs (example doesn't really show what it's for), and color<br>
> (just an "examples" heading, no actual examples.<br>
> <a href="https://github.com/ipython/ipython/pull/226/files" target="_blank">https://github.com/ipython/ipython/pull/226/files</a><br>
> - save: Clean diff, but the example and explanation aren't very clear.<br>
> Probably needs to be redone from scratch.<br>
> <a href="https://github.com/ipython/ipython/pull/225" target="_blank">https://github.com/ipython/ipython/pull/225</a><br>
> - cpaste: Appears to have ripped out the actual function while writing the<br>
> docs, and stitched its definition onto the magic_paste function code.<br>
> Example could also be a bit clearer.<br>
> <a href="https://github.com/ipython/ipython/pull/224" target="_blank">https://github.com/ipython/ipython/pull/224</a><br>
> - colors: Same changes (& problems) as request 229.<br>
> <a href="https://github.com/ipython/ipython/pull/223" target="_blank">https://github.com/ipython/ipython/pull/223</a><br>
> - timeit: Too many equivalent examples.<br>
> <a href="https://github.com/ipython/ipython/pull/222/files" target="_blank">https://github.com/ipython/ipython/pull/222/files</a><br>
> - alias: Already had examples, changes remove one of them. Adds two, but one<br>
> (ls) is available without aliasing, and the other is inaccurate (the example<br>
> shows the result of a man command).<br>
> <a href="https://github.com/ipython/ipython/pull/221" target="_blank">https://github.com/ipython/ipython/pull/221</a><br>
> - pycat: Doesn't actually use a Python file, so rather defeats the point of<br>
> pycat. Not sure that an example is really helpful, as it doesn't show the<br>
> colouring you get at a terminal. <a href="https://github.com/ipython/ipython/pull/219" target="_blank">https://github.com/ipython/ipython/pull/219</a><br>
> - hist: Formatting is off (probably related to a bug in the code). Also, the<br>
> "this is an example of..." line seems redundant.<br>
> <a href="https://github.com/ipython/ipython/pull/218/files" target="_blank">https://github.com/ipython/ipython/pull/218/files</a><br>
><br>
> Whew. Of these, I recommend merging 228 (%who), and closing 225 (%save), 232<br>
> (%profile) with "try again" and 227 (%logon), for which I'm not sure that<br>
> it's even possible to do an example.<br>
><br>
> The others will need further work. Fernando, any sort of feel for how many<br>
> of the SciPy India contributors will still be in the loop to revise their<br>
> contributions?<br>
<br>
</div></div>Thanks for going through all of this! Unfortunately due to the delay,<br>
and the fact that many of them were first-timers to pretty much<br>
everything, I'm not too hopeful. I think if there are some where we<br>
can easily finish up what they did with minimal work, it would be a<br>
nice gesture. But those that require significant rework, just<br>
dropping a comment would be sufficient. If they do pop up again and<br>
come back with improvements, then we can engage them better for the<br>
future.<br>
<br>
In my defense, I was trying to handle a sprint where 60! people wanted<br>
to contribute, most of which had never coded a single line of Python.<br>
So I tried to do something where at least they would learn during the<br>
day, even if it wasn't realistic to expect actual final contributions<br>
to the codebase to come from everyone.<br>
<div><div></div><div class="h5"><br>
> Looking at the others:<br>
><br>
> Small pull requests:<br>
> - %hist output format: I've solved the same problem in a different way in<br>
> pull request 261, see below. <a href="https://github.com/ipython/ipython/pull/236" target="_blank">https://github.com/ipython/ipython/pull/236</a><br>
> - Logging unicode characters (0.10 series): Good start. See my comment there<br>
> for minor concern. <a href="https://github.com/ipython/ipython/pull/249" target="_blank">https://github.com/ipython/ipython/pull/249</a><br>
> - check_gtk: Removes a function. Mark Voorhies has tested the change and<br>
> commented that it works. I'll admit to not understanding what it's about.<br>
> <a href="https://github.com/ipython/ipython/pull/237" target="_blank">https://github.com/ipython/ipython/pull/237</a><br>
> - Typos in zmq/blockingkernelmanager.py: Seems OK to me.<br>
> <a href="https://github.com/ipython/ipython/pull/203/files" target="_blank">https://github.com/ipython/ipython/pull/203/files</a><br>
> - cProfiling ipython.py: Updated in response to Brian's concerns. Needs<br>
> reviewing again. <a href="https://github.com/ipython/ipython/pull/193" target="_blank">https://github.com/ipython/ipython/pull/193</a><br>
> - Autocomplete in Emacs: Removes the all_completions function (which<br>
> apparently was broken). One other person says it works. Needs review.<br>
> <a href="https://github.com/ipython/ipython/pull/204" target="_blank">https://github.com/ipython/ipython/pull/204</a><br>
> - Unicode issues [My changes]: Tests now integrated into main test suite<br>
> (thanks to help from Robert Kern). Needs review.<br>
> <a href="https://github.com/ipython/ipython/pull/252" target="_blank">https://github.com/ipython/ipython/pull/252</a><br>
> - Wildcard module [My changes]: Fixes issue 129. The bug and the fix are<br>
> pretty much self contained, so shouldn't interfere with changes in other<br>
> code. No feedback yet. (this is pushing what I'd call small, but shouldn't<br>
> change functionality besides fixing the bug)<br>
> <a href="https://github.com/ipython/ipython/pull/251" target="_blank">https://github.com/ipython/ipython/pull/251</a><br>
><br>
> Medium pull requests:<br>
> - Fixing up various magic commands with the new history system [My changes]:<br>
> Revised after Fernando's comments, unit tests added. Conflicts with<br>
> Madhusudan's #203, see below. <a href="https://github.com/ipython/ipython/pull/261" target="_blank">https://github.com/ipython/ipython/pull/261</a><br>
> - %hist command bugs: This solves one of the same problems I've fixed in<br>
> #261, in a different way. <a href="https://github.com/ipython/ipython/pull/203/files" target="_blank">https://github.com/ipython/ipython/pull/203/files</a><br>
> - Unbundling external libraries: Revised in response to Fernando's and<br>
> Brian's concerns. Needs reviewing again.<br>
> <a href="https://github.com/ipython/ipython/pull/191" target="_blank">https://github.com/ipython/ipython/pull/191</a><br>
> - Pyside compatibility: Waiting for contributor to resolve bug with SVG<br>
> handling. <a href="https://github.com/ipython/ipython/pull/259" target="_blank">https://github.com/ipython/ipython/pull/259</a><br>
> - Kernel logging: Fernando has requested a final check from Robert.<br>
> <a href="https://github.com/ipython/ipython/pull/264" target="_blank">https://github.com/ipython/ipython/pull/264</a><br>
><br>
> Larger pull requests:<br>
> - Magic arguments decorators. Updated in response to Fernando's comments.<br>
> Needs reviewing again. <a href="https://github.com/ipython/ipython/pull/199" target="_blank">https://github.com/ipython/ipython/pull/199</a><br>
><br>
> Not quite yet:<br>
> - HTML Notebook: Gained a lot of interest, but not quite ready yet. Possible<br>
> project for GSoC this year. (But if we're going to accumulate alternative<br>
> frontends, should we consider putting each frontend in its own repo?)<br>
> <a href="https://github.com/ipython/ipython/pull/179" target="_blank">https://github.com/ipython/ipython/pull/179</a><br>
> - MinRK's newparallel: Work in progress. Lots of<br>
> work.<a href="https://github.com/ipython/ipython/pull/254" target="_blank">https://github.com/ipython/ipython/pull/254</a><br>
<br>
</div></div>Ok, thanks again for this analysis.<br>
<br>
Over the next few days, I suggest that anyone willing to 'take<br>
ownership' of one just pitch in and add a comment to the corresponding<br>
one, mentioning they'll keep an eye on it. That way, we can split<br>
this load and claw our way back to a small number of pending ones.<br>
<br>
Best,<br>
<br>
f<br>
_______________________________________________<br>
IPython-dev mailing list<br>
<a href="mailto:IPython-dev@scipy.org">IPython-dev@scipy.org</a><br>
<a href="http://mail.scipy.org/mailman/listinfo/ipython-dev" target="_blank">http://mail.scipy.org/mailman/listinfo/ipython-dev</a><br>
</blockquote></div><br></div>