[Patches] [ python-Patches-1669633 ] methods for bytes

[SourceForge.net]

Patches item #1669633, was opened at 2007-02-26 19:04
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1669633&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 3000
>Status: Closed
>Resolution: Accepted
Priority: 5
Private: No
Submitted By: Pete Shinners (shredwheat)
Assigned to: Neal Norwitz (nnorwitz)
Summary: methods for bytes

Initial Comment:
This adds the methods for bytes objects as listed in PEP 358. It includes tests for the newly added methods.


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

>Comment By: Neal Norwitz (nnorwitz)
Date: 2007-02-27 11:02

Message:
Logged In: YES 
user_id=33168
Originator: NO

Thanks Pete!

Committed revision 54000.


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

Comment By: Pete Shinners (shredwheat)
Date: 2007-02-27 08:41

Message:
Logged In: YES 
user_id=1076442
Originator: YES

Attached is the updated patch. This should apply clean after the "fromhex"
patch that was recently applied. This ignores the previous whitespace
cleanup patch.

Most of this code was copied from string. Minor modifications were made on
these, but no real code changes. The following list of new methods are
mostly modelled after list methods.
    extend, insert, append, reverse, pop, remove

This adds an Init method to match the Fini. Also added a comment stating
partition is the only dependency of all this init. If partiion is removed
from bytes, so should this global stuff.

_adjust_indices was copied directly from str. This is not used anywhere
else in the codebase, so it should be unique again when str is removed.
Added comment about this.

Switched new single argument methods to METH_O.

The 0-255 range is enforced on insert, append, and remove. Outside results
in ValueError.
File Added: bytesmethods2.diff

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

Comment By: Pete Shinners (shredwheat)
Date: 2007-02-26 23:28

Message:
Logged In: YES 
user_id=1076442
Originator: YES

This is a split of the whitespacing. This matches the PEP7 style guide for
4-space indentation in C code.



File Added: bytesspace.diff

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

Comment By: Neal Norwitz (nnorwitz)
Date: 2007-02-26 21:35

Message:
Logged In: YES 
user_id=33168
Originator: NO

Thanks for the patch.

I'd like to try to focus on reviewing only new code.  How much of this
code was copied straight from string, how much was modified and how much
was new?  

There are a bunch of formatting changes.  Could you revert those portions
and add them to a separate patch (the file can be attached here if you
want).

Since you are adding a Fini method, I wonder if you ought to add a
corresponding Init method.  This would init nullbytes and save a comparison
on each instantiation of a bytes object.

_adjust_indices() looks generic, does this code exist elsewhere/can it be
reused?

All the methods that take a single argument should use METH_O (e.g.,
bytes_extend).

Shouldn't value in bytes_remove(), bytes_insert(), and possibly elsewhere
be limited to 0..255?  If the value is > 255, it will traverse the entire
bytes, then raise an exception rather than fail early.



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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1669633&group_id=5470