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

noreply@sourceforge.net noreply@sourceforge.net
Tue, 29 Aug 2000 07:53:08 -0700


Patch #100902 has been updated. 

Project: 
Category: core (C code)
Status: Postponed
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.

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

Date: 2000-Aug-28 01:02
By: twouters

Comment:
Okay, but I have one question: WTH do you mean by '2.' ? assert what size ? the return value of getlenofrange ? all those values are passed-in, from Python expressions, so I don't think assert()in them is that good an idea. I really doubt you want the program to abort() if a user types in '[:0:]' or some such.

Oh, and on the nit: I still don't agree, see my other comment in, uhm, some patch or other: we aren't using up a scarce resource with the MAGIC, as it's created by date, and it prevents *new* bytecode from running in *old* (relatively) interpreters, even if those old interpreters are only a few days old and the whole thing would 'only' happen to other developers. Or is there something else I don't know about ? 

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

Date: 2000-Aug-28 01:32
By: tim_one

Comment:
Ack, 'twas a typo, I meant

 assert(step != 0);

It's an *internal* (not user!) error if anyone calls this function with a zero step, so the function should at least assert that its caller isn't screwing up.  The advantage to an assert over SystemError is simplicity and that there's no runtime cost in release builds.  C functions should always verify their preconditions in debug builds, and public C functions regardless of build mode.

The magic number isn't worth arguing about it -- leave it in, take it out, both OK by me (that's what "nit" means <wink>).  I always delete my .pyc/.pyo files after an update changes anything; I assume everyone does.

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

Date: 2000-Aug-28 13:30
By: twouters

Comment:
Ok, bettah version, that should address all Tim's issues. And I dropped the Upping of the ByteCodeMagic too.


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

Date: 2000-Aug-28 13:53
By: gvanrossum

Comment:
Oops. Sorry. I'm getting second thoughts about this. I'll post to python-dev.
-------------------------------------------------------

Date: 2000-Aug-28 21:32
By: tim_one

Comment:
Assigned to Guido.  Please Reject or assign back to me (I quickly scanned Thomas's changes, and *think* they're spot-on, but if this is going in I want to spend a little more time with the patch before Accept'ing it).
-------------------------------------------------------

Date: 2000-Aug-29 07:53
By: gvanrossum

Comment:
Postponed - this may still happen but definitely not in 2.0, there's too much doubt about whether this is the right syntax and too many alternative proposals that have to be reviewed. (Well, at least one. :-)
-------------------------------------------------------

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

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