[Patches] [Patch #100902] range-literals, not relative to listcomprehensions

noreply@sourceforge.net noreply@sourceforge.net
Mon, 28 Aug 2000 00:31:19 -0700


Patch #100902 has been updated. 

Project: 
Category: core (C code)
Status: Open
Summary: range-literals, not relative to listcomprehensions

Follow-Ups:

Date: 2000-Jul-15 16:31
By: twouters

Comment:
A new version of the patch that adds range-literals ([0:20], [:33:2], etc), this time not relative to list-comprehensions. I marked the previous one 'Out of Date'. I'm still working on the PEP, but I think the patch is done ;-)


-------------------------------------------------------

Date: 2000-Jul-18 08:25
By: twouters

Comment:
Someone needs to check this patch out, and tell me whether Guido really meant 'check it in' by his 'Right!' in http://www.python.org/pipermail/python-dev/2000-July/013234.html

Tim, you seem like the perfect person, especially now you're sharing half a room with Guido ;)

-------------------------------------------------------

Date: 2000-Jul-22 15:04
By: tim_one

Comment:
Mostly adding a comment to see whether SourceForge generates patches-list email.
From a quick scan, please update the patch so that new functions are already ANSIfied.  This also needs doc changes.

-------------------------------------------------------

Date: 2000-Jul-23 06:23
By: twouters

Comment:
New version of patch, ANSIfied and all. Documentation, uh, comes later. Promise ;)

-------------------------------------------------------

Date: 2000-Aug-13 10:45
By: twouters

Comment:
Up-to-date version, which thus is again relative to list comprehensions, since they are checked in :-) Working on documentation, will be added later.

-------------------------------------------------------

Date: 2000-Aug-13 10:45
By: twouters

Comment:
Up-to-date version, which thus is again relative to list comprehensions, since they are checked in :-) Working on documentation, will be added later.

-------------------------------------------------------

Date: 2000-Aug-15 11:49
By: tim_one

Comment:
Tim, get off your fat ass and review this!  Umm, OK.

Thomas, we need the doc changes Soon or I'll have to postpone this.  Guido already likes the idea, so there's nothing to stop this from getting accepted except a complete patch (and, ya, getting that fat-ass Tim to actually review it!).
-------------------------------------------------------

Date: 2000-Aug-15 14:08
By: twouters

Comment:
Here are the doc changes. Not much, I'm afraid, because I'm not sure where or how to insert them, assides from where I added them now (that being the reference manual.) I can write a bit of text for the tutorial, if that's desired, but I'll need some hints as to where, how long, in what style, and how this 'TeX' thing works.



-------------------------------------------------------

Date: 2000-Aug-15 14:32
By: tim_one

Comment:
Great!  Please work directly with Fred Drake on doc issues.  Keep your docs simple plain ASCII and he's a whiz at adding the TeX bit later.  And when he's happy with the docs, everyone is.
-------------------------------------------------------

Date: 2000-Aug-22 09:13
By: twouters

Comment:
New version, one that actually includes the minimal docs I wrote :-) Also up to date wrt. the latest Grammar changes and such.

-------------------------------------------------------

Date: 2000-Aug-27 12:28
By: tim_one

Comment:
Thomas, could you please update this patch?  Running patch on Windows is always an adventure, so I had Guido check that this fails for him too on import.c (probably just the magic number) and ref5.tex.  Guido also reports that his patch rejected the graminit.h change, but mine didn't (I'm guessing he modified his graminit.h locally, though; mine is straight from CVS).  Thanks!
-------------------------------------------------------

Date: 2000-Aug-27 14:04
By: twouters

Comment:
Alright, here's an updated version. I also included graminit.h and .c, because I don't know if you can run pgen, and if you complain about graminit.h not patching, I guess you can't ;-)

(the fact that graminit.h was included in the previous patch was an honest mistake. But if you intend to read this patch: there's nothing interesting below graminit.c, which is about 900 lines.)

-------------------------------------------------------

Date: 2000-Aug-28 00:31
By: tim_one

Comment:
Assigned to Fred to eyeball the docs; assign back to me if you're happy, else to Thomas if you're not.

Thomas, this looks good!  I have a nit and a complaint.  

The nit is that we don't need to keep bumping import's magic number -- once per public release should be enough.

The complaint is that PyList_GetLenOfRange is brittle.  In the context the code originally appeared, it was file-private, so all call sites were known and close by, that its assumptions were met was easy to verify, and that it did only 48% of the job didn't matter.  But here it's almost part of the public API:  it should do the whole job.  By that I mean it should verify that its step argument is non-zero, and if the step argument is less than 0 it should shuffle the input values around to compute the correct answer.  As is, this delicate shuffling is duplicated in its callers, and if someone happens to pass in a step < 0 it's just hosed.

I would prefer:

1. Rename it with a leading underscore.  This doesn't seem useful enough to be in the public API.

2. If it's private, it's OK to assert argument preconditions instead of error-checking them.  So stick in
assert(size != 0).

3. Deal with negative step size internally.  Like (leading x to stop SourceForge from left-flushing all the lines):

if (step < 0) {
x    long temp = lo;
x    lo = hi;
x    hi = temp;
x    step = -step;
}

right after the new assert.

4. Remove the if/else argument swapping from all its callers.

5. Change the comment before the function to match.

-------------------------------------------------------

-------------------------------------------------------
For more info, visit:

http://sourceforge.net/patch/?func=detailpatch&patch_id=100902&group_id=5470