[Python-ideas] Tulip patches

[Guido van Rossum]

On Fri, Dec 21, 2012 at 1:59 PM, Geert Jansen <geertj at gmail.com> wrote:
> On Fri, Dec 21, 2012 at 4:57 PM, Guido van Rossum <guido at python.org> wrote:
>
>> This is a fine place, but you would make my life even easier by
>> uploading the patches to codereview.appspot.com, so I can review them
>> and send comments in-line.
>
> I tried to get Tulip added as a new repository there, but i'm probably
> doing something wrong.. In the mean time i'm sending my updated
> patches below..

Yeah, sorry, the upload form is not to be used. You should use the
upload.py utility instead:
https://codereview.appspot.com/static/upload.py

>> I've given you checkin permissions. Please send a contributor form to
>> the PSF (http://www.python.org/psf/contrib/contrib-form/).
>
> Done!
>
>>> 0001-run-fd-callbacks.patch
> [...]
>> Interesting. Go ahead and submit.
> [from your other email]
>> Whoa! I just figured out the problem. You don't have to run the ready
>> queue twice. You just have to set the poll timeout to 0 if there's
>> anything in the ready queue. Please send me an updated patch before
>> submitting.
>
> New patch attached.

Looks good to me. Check it in!

>>> 0002-call-every-iteration.patch
> [...]
>> There's one odd thing here: you remove cancelled everytime handlers
>> *after* already scheduling them. It would seem to make more sense to
>> schedule them first. Also, a faster way to do this would be
>>
>>     self._everytime = [handler in self._everytime if not handler.cancelled]
>>
>> (Even if you iterate from the back, remove() is still O(N), so if half
>> the handlers are to be removed, your original code would be O(N**2).)
>
> ACK regarding the comment on O(N^2). The reason i implemented it like
> this is that i didn't want to regenerate the list at every iteration
> of the loop (maybe i'm unduly worried though...). The attached patch
> does as you suggest but only in case there are cancelled handlers.

LG, except:

- Maybe rename 'cancelled' to 'any_cancelled'.
- PEP 8 conformance: [foo bar], not [ foo bar ].

You can check it in after fixing those issues.

>> PS. If you want to set up a mailing list or other cleverness I can set
>> you up as a project admin. (I currently have all patches mailed to me
>> but we may want to set up a separate list for that.)
>
> I'm happy to be an admin and set up a Google Groups for this.

Made you an admin. Go ahead.

> On the
> other hand, tulip is supposed to become part of the standard library,
> right? Maybe python-dev is as a good place to discuss tulip? Your
> call..

I think it's too soon to flood python-dev with every little detail
(though I just did post there about the PEP).

> I'll go ahead and commit the two trivial patches, and wait for your
> ACK on the updated versions of the other two.

Thanks!

-- 
--Guido van Rossum (python.org/~guido)