classification
Title: bytes.fromhex()
Type: Stage:
Components: Interpreter Core Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: nnorwitz Nosy List: georg.brandl, nnorwitz
Priority: normal Keywords: patch

Created on 2007-02-26 19:18 by georg.brandl, last changed 2007-02-27 08:42 by georg.brandl. This issue is now closed.

Files
File name Uploaded Description Edit
bytes-fromhex.diff georg.brandl, 2007-02-26 19:18
Messages (3)
msg51969 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2007-02-26 19:18
Implement bytes.fromhex().
msg51970 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2007-02-27 06:41
I don't know if this is supposed to be in the API or not.  I'm too tired to care, but here are my comments on the code.

It would be better to use a variable name other than new, since new is a keyword in C++.  isspace() checks for any whitespace, so not only can there be spaces, there can be tabs, carriage returns, new lines, etc.  I don't know if the code is correct or the comment is. I see you have a test for other whitespace, so I would just update the comment to say _any_ whitespace.

Does byteslen need the + 1?

Is there a macro to get ob_bytes rather than accessing directly?
    buf = ((PyBytesObject *)new)->ob_bytes;

It would be good to add the index position (i) in the error msg.  It might be hard to see/non-obvious from the 2 chars (or potentially ambiguous).

How about adding a test that passes a buffer('') and buffer('00')?

These are all nits.  Assuming Guido is fine with the API, I'd say clean it up and check it in.
msg51971 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2007-02-27 08:42
It is in the PEP, so I assume it is in the API.

I intended for all spaces to be allowed, but the PEP says otherwise,
so I updated the implementation accordingly.

I fixed the rest of your nits, fixed another bug and checked it in as rev. 53989.
History
Date User Action Args
2007-02-26 19:18:47georg.brandlcreate