Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyBytes (buffer) .extend method needs to accept any iterable of ints #45624

Closed
gpshead opened this issue Oct 16, 2007 · 9 comments
Closed

PyBytes (buffer) .extend method needs to accept any iterable of ints #45624

gpshead opened this issue Oct 16, 2007 · 9 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@gpshead
Copy link
Member

gpshead commented Oct 16, 2007

BPO 1283
Nosy @gpshead, @avassalotti
Files
  • byte_extend.patch
  • byte_extend-2.patch
  • byte_extend-3.patch
  • byte_extend-4.patch
  • byte_extend-5.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/avassalotti'
    closed_at = <Date 2007-12-04.05:51:36.612>
    created_at = <Date 2007-10-16.00:31:50.311>
    labels = ['interpreter-core', 'type-feature']
    title = 'PyBytes (buffer) .extend method needs to accept any iterable of ints'
    updated_at = <Date 2008-01-06.22:29:45.542>
    user = 'https://github.com/gpshead'

    bugs.python.org fields:

    activity = <Date 2008-01-06.22:29:45.542>
    actor = 'admin'
    assignee = 'alexandre.vassalotti'
    closed = True
    closed_date = <Date 2007-12-04.05:51:36.612>
    closer = 'alexandre.vassalotti'
    components = ['Interpreter Core']
    creation = <Date 2007-10-16.00:31:50.311>
    creator = 'gregory.p.smith'
    dependencies = []
    files = ['8847', '8848', '8849', '8856', '8857']
    hgrepos = []
    issue_num = 1283
    keywords = ['patch']
    message_count = 9.0
    messages = ['56478', '58072', '58073', '58074', '58106', '58107', '58108', '58109', '58177']
    nosy_count = 2.0
    nosy_names = ['gregory.p.smith', 'alexandre.vassalotti']
    pr_nums = []
    priority = 'low'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue1283'
    versions = ['Python 3.0']

    @gpshead
    Copy link
    Member Author

    gpshead commented Oct 16, 2007

    The PyBytes (PEP-3137 buffer) .extend() method currently only accepts as
    input something supporting the PEP-3118 buffer API. It also needs to
    accept an iterable of ints in the 0..255 range.

    @tiran tiran added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Nov 8, 2007
    @avassalotti
    Copy link
    Member

    Here a patch that adds support for any iterable (or sequence) of
    integers to bytearray.extend().

    @avassalotti avassalotti self-assigned this Dec 2, 2007
    @avassalotti
    Copy link
    Member

    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.

    @avassalotti
    Copy link
    Member

    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.

    @avassalotti
    Copy link
    Member

    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().

    @gpshead
    Copy link
    Member Author

    gpshead commented Dec 3, 2007

    "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().

    @avassalotti
    Copy link
    Member

    Done. Is there any other issue with the patch?

    @gpshead
    Copy link
    Member Author

    gpshead commented Dec 3, 2007

    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\>


    @avassalotti
    Copy link
    Member

    Thank you Gregory for the review!

    Committed to r59314.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants