classification
Title: Move the special-case for integer objects out of PyBytes_FromObject.
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.2
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, benjamin.peterson, mark.dickinson
Priority: normal Keywords: 26backport, patch

Created on 2009-08-11 21:15 by alexandre.vassalotti, last changed 2009-12-31 03:57 by alexandre.vassalotti. This issue is now closed.

Files
File name Uploaded Description Edit
move_int_special_case.diff alexandre.vassalotti, 2009-08-11 21:15
Messages (7)
msg91483 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-08-11 21:15
The documentation for PyBytes_FromObject states:

.. cfunction:: PyObject* PyBytes_FromObject(PyObject *o)

   Return the bytes representation of object *o* that implements
   the buffer protocol.

However, there exists a special-case for integer object in
PyBytes_FromObject that makes the function return a null-initialized
bytes object. Currently, this is only used for handling `bytes(10)'.

I don't like changing APIs after a stable release, but I believe this
behaviour is error-prone and surprising (and darn right annoying even).
So, I believe the special-case should be made specific to the bytes
constructor.

Thus, here is the fine patch.
msg91484 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-08-11 21:18
Oh, in case you wonder, the added PyUnicode_Check(x) check is to force
PyBytes_FromObject to raise an error when given an empty unicode string
(I will this as a comment in my patch).
msg91485 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-08-11 21:25
+1
msg91556 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-08-14 16:12
+1 from me too;  I agree the current API of PyBytes_FromObject is ugly.  

Is there still a need for a separate C function for creating a zero-
initialized bytes object from a Py_ssize_t or a Python integer?  It's a 
fairly simple operation (PyBytes_FromStringAndSize + memset), so perhaps 
it doesn't need its own public function.

I'm not sure about the extra PyUnicode_Check:  this seems to go against 
Python's philosophy of duck-typing.  After all, the empty string *is* an 
iterable all of whose elements are integers.  And this check doesn't cover 
other, similar, cases:  for example, list('') will still be converted by 
PyBytes_FromObject, while list('123') won't.  What's the benefit?

If this check is going to stay, there should probably also be a unit test 
for this behaviour.

Apart from the reservation about the PyUnicode_Check, the patch looks good 
to me.  All tests pass on my machine (OS X 10.5) with this patch applied.
msg96255 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-12-11 13:54
Mark Dickinson wrote:
> Is there still a need for a separate C function for creating a zero-
> initialized bytes object from a Py_ssize_t or a Python integer?

What C function are you referring to?

> And this check doesn't cover other, similar, cases: for example, list('')
> will still be converted by PyBytes_FromObject, while list('123') won't.

Well, one is a just empty list and the other a list of strings. I don't see
why converting a empty list to bytes should raise an error. Although I agree
the type check is a bit out of place, I think it will help prevents bugs. 

In addition, PyBytes_FromObject() is documented as equivalent to the
built-in
bytes(). Since calling bytes() with any unicode string raises a TypeError
exception, unless an encoding is specified, I believe PyBytes_FromObject()
should also follow this convention.
msg96264 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-12-11 16:46
> What C function are you referring to?

A currently non-existent one!  I was wondering whether the current bytes-
from-integer functionality that PyBytes_FromObject supplies is important 
enough to preserve somehow.  I'd guess not.
msg97077 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2009-12-31 03:57
Committed in r77174. Thank you for reviewing!
History
Date User Action Args
2009-12-31 03:57:04alexandre.vassalottisetstatus: open -> closed
resolution: accepted
messages: + msg97077

stage: patch review -> resolved
2009-12-11 16:46:02mark.dickinsonsetmessages: + msg96264
2009-12-11 13:54:20alexandre.vassalottisetmessages: + msg96255
2009-11-14 20:38:10alexandre.vassalottilinkissue1023290 dependencies
2009-08-14 16:12:19mark.dickinsonsetnosy: + mark.dickinson
messages: + msg91556
2009-08-11 21:25:19benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg91485
2009-08-11 21:18:12alexandre.vassalottisetmessages: + msg91484
2009-08-11 21:15:31alexandre.vassalotticreate