| msg66042 - (view) |
Author: Benjamin Peterson (benjamin.peterson) *  |
Date: 2008-05-01 20:48 |
| Per discussions on Python-3000, I've stipped range down to a bare minimum. Here's an overview of the patch: 1. No slicing. 2. Length is computed in constructor and is a PyLong in the object's struct. __len__ simply tries to convert it to a Py_ssize_t. 3. start, stop, and, step are exposed as attributes |
|
|
| msg66049 - (view) |
Author: Facundo Batista (facundobatista) * |
Date: 2008-05-01 21:39 |
| Just for the record: Why __len__ tries to convert it to Py_size_t (possibly generating an Overflow), and not just returns the PyLong object? |
|
|
| msg66050 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-01 21:41 |
| __len__ always has to return a Py_ssize_t because that's the data type that represents lengths in C. It's just the way it is. |
|
|
| msg66060 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2008-05-02 02:38 |
| The start/step/stop getter functions should INCREF return values. Is there a reason not to adapt slice implementation: static PyMemberDef slice_members[] = { {"start", T_OBJECT, offsetof(PySliceObject, start), READONLY}, {"stop", T_OBJECT, offsetof(PySliceObject, stop), READONLY}, {"step", T_OBJECT, offsetof(PySliceObject, step), READONLY}, {0} }; |
|
|
| msg66061 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-02 02:49 |
| On Thu, May 1, 2008 at 9:38 PM, Alexander Belopolsky <report@bugs.python.org> wrote: > > Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment: > > The start/step/stop getter functions should INCREF return values. Is > there a reason not to adapt slice implementation: No, its much simpler. |
|
|
| msg66063 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2008-05-02 03:43 |
| - With length precomputed in range_new, we can probably get rid of get_len_of_range. - There is no need to recompute length in range_iter. A nit: iterator object's size member is called "len", but the same member of the range object is called "length." I have no preference between the two names, but I think they should be the same. Off-topic: why optimized iterator uses long rather than ssize_t arithmetics? On x86-64, ssize_t is 64 and long is 32 bit. |
|
|
| msg66099 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2008-05-02 17:31 |
| One more nit: you don't need to zero out trailing range_as_sequence members explicitly. static PySequenceMethods range_as_sequence = { (lenfunc)range_length, /* sq_length */ }; should be enough. |
|
|
| msg66104 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2008-05-02 19:08 |
| See also comments published on the code review site: http://codereview.appspot.com/602 |
|
|
| msg66109 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-02 20:56 |
| Attaching a new patch from reviews. __len__ has been removed. I'll post it to Guido's Codereview tool when it's active (I'm getting 500 server errors when I try to upload.) |
|
|
| msg66111 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-02 21:11 |
| Are you sure you want to remove len of range? It breaks quite a few tests. |
|
|
| msg66116 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2008-05-02 21:47 |
| Show me which tests break and I'll decide. |
|
|
| msg66123 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2008-05-02 22:17 |
| Does this/will this supercede http://bugs.python.org/issue2690 ? |
|
|
| msg66126 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-02 22:31 |
| 16 tests failed: test_ctypes test_datetime test_deque test_enumerate test_heapq test_itertools test_list test_mutants test_operator test_pickle test_pickletools test_random test_richcmp test_set test_trace test_userlist |
|
|
| msg66128 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2008-05-02 22:46 |
| We need to look at more details why those fail. Perhaps there's one common place where this is used? Did you add __length_hint__ yet? |
|
|
| msg66130 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-02 23:11 |
| On Fri, May 2, 2008 at 5:46 PM, Guido van Rossum <report@bugs.python.org> wrote: > > Guido van Rossum <guido@python.org> added the comment: > > We need to look at more details why those fail. Perhaps there's one > common place where this is used? Did you add __length_hint__ yet? I added __length_hint__ in the last patch. Many of the failures are the result of failings in seq_tests.py. As test_random points out, you can't pick a random integer from a set. Some of these failures are also the result of removing range slicing. (I'm not proposing we bring that back, though.) Overall though, I think it's more practical to have a length implementation that works in 99.78% cases than not and force people to to convert it to a list first. |
|
|
| msg66136 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2008-05-03 00:19 |
| Fair enough. Let's keep __len__. Did you upload the patch to the codde review site yet? Hopefully I have time to look at it tonight. |
|
|
| msg66139 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-03 02:15 |
| I brought __len__ back, and tried to fix the tests failing because they were indexing range. I couldn't figure out how to fix test_itertools. (It's on codereview, too). |
|
|
| msg66159 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-05-03 17:20 |
| Address more concerns with attached patch. |
|
|
| msg68258 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-06-16 02:44 |
| Guido, is it safe to safe this isn't going to happen? |
|
|
| msg68260 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2008-06-16 03:22 |
| Yeah, let's just reject this. There doesn't appear to be much demand. |
|
|