classification
Title: PyBytes (buffer) .extend method needs to accept any iterable of ints
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: alexandre.vassalotti Nosy List: alexandre.vassalotti, gregory.p.smith
Priority: low Keywords: patch

Created on 2007-10-16 00:31 by gregory.p.smith, last changed 2008-01-06 22:29 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
byte_extend.patch alexandre.vassalotti, 2007-12-02 00:12
byte_extend-2.patch alexandre.vassalotti, 2007-12-02 00:41
byte_extend-3.patch alexandre.vassalotti, 2007-12-02 01:10
byte_extend-4.patch alexandre.vassalotti, 2007-12-02 23:17
byte_extend-5.patch alexandre.vassalotti, 2007-12-03 03:18
Messages (9)
msg56478 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2007-10-16 00:31
The PyBytes (pep3137 buffer) .extend() method currently only accepts as
input something supporting the pep3118 buffer API.  It also needs to
accept an iterable of ints in the 0..255 range.
msg58072 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2007-12-02 00:12
Here a patch that adds support for any iterable (or sequence) of
integers to bytearray.extend().
msg58073 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2007-12-02 00:41
Made 2 minor enhancements to the patch:
   + Added the proper type-cast to PyMem_Realloc call.
   + Changed (len >> 1) to (len >> 1) + 1, just to be sure that the
buffer  doesn't overflow if __length_hint__ return 0 or 1 erroneously.
msg58074 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2007-12-02 01:10
There is a reference leak in my previous patches. So, I updated (again)
the patch. There is still another possible leak if the PyMem_Realloc
return NULL (i.e., the system is out of memory), but I don't think it
worth fixing.
msg58106 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2007-12-02 23:17
Here yet another revision of the patch. This one makes
bytearray.extend()   try to determine the length of its argument a bit
more aggressively -- i.e., also uses PyObject_Length().
msg58107 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2007-12-03 02:53
"There is still another possible leak if the PyMem_Realloc
return NULL (i.e., the system is out of memory), but I don't think it
worth fixing."

Do it.  It looks easy: a Py_DECREF(it) before the return PyErr_NoMemory().
msg58108 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2007-12-03 03:18
Done. Is there any other issue with the patch?
msg58109 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2007-12-03 03:50
reading 5.patch over...

Any particular reason for using buf_size = 32 when the length isn't
known up front?  add a comment saying why (or that its just a magic
guess).  anyways it sounds like a fine starting value.  picking
anything "better" would require profiling.

perhaps use a list comprehension instead of map() in the unit test?
either works, its a style thing.

  (int(x) for x in orig*50)    [int(x) for x in orig*50]

also the uses of 5 and -5 in that test could be written using
len(orig) instead of 5.

add another assertRaises that tests to make sure a list with -1 in it
raises a ValueError.

While I dislike that this code makes a temporary copy of the data
first, doing otherwise is more complicated so the simplicity of this
one wins.  Leave that optimization for later.  Your code looks good.

-gps

On 12/2/07, Alexandre Vassalotti <report@bugs.python.org> wrote:
>
> Alexandre Vassalotti added the comment:
>
> Done. Is there any other issue with the patch?
>
> Added file: http://bugs.python.org/file8857/byte_extend-5.patch
>
> __________________________________
> Tracker <report@bugs.python.org>
> <http://bugs.python.org/issue1283>
> __________________________________
>
msg58177 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2007-12-04 05:51
Thank you Gregory for the review!

Committed to r59314.
History
Date User Action Args
2008-01-06 22:29:45adminsetkeywords: - py3k
versions: Python 3.0
2007-12-04 05:51:36alexandre.vassalottisetstatus: open -> closed
resolution: accepted
messages: + msg58177
2007-12-03 03:50:18gregory.p.smithsetmessages: + msg58109
2007-12-03 03:18:38alexandre.vassalottisetfiles: + byte_extend-5.patch
messages: + msg58108
2007-12-03 02:53:44gregory.p.smithsetmessages: + msg58107
2007-12-02 23:17:26alexandre.vassalottisetfiles: + byte_extend-4.patch
messages: + msg58106
2007-12-02 01:10:25alexandre.vassalottisetfiles: + byte_extend-3.patch
messages: + msg58074
2007-12-02 00:41:52alexandre.vassalottisetfiles: + byte_extend-2.patch
messages: + msg58073
2007-12-02 00:12:36alexandre.vassalottisetfiles: + byte_extend.patch
nosy: + alexandre.vassalotti
messages: + msg58072
assignee: alexandre.vassalotti
keywords: + patch
resolution: accepted -> (no value)
2007-11-08 13:43:25christian.heimessetpriority: low
type: enhancement
resolution: accepted
components: + Interpreter Core
2007-10-16 00:31:50gregory.p.smithcreate