classification
Title: bytearray methods center, ljust, rjust don't accept a bytearray as the fill character
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: docs@python, eli.bendersky, haypo, ncoghlan, petri.lehtinen, pitrou, py.user, python-dev, r.david.murray, rhettinger, terry.reedy
Priority: normal Keywords: needs review, patch

Created on 2011-06-21 03:50 by py.user, last changed 2011-07-30 04:13 by eli.bendersky. This issue is now closed.

Files
File name Uploaded Description Edit
c_format_bytearray.patch petri.lehtinen, 2011-07-26 11:38 Allow bytearray for 'c' format review
c_format_bytearray_plus_additional_tests.patch petri.lehtinen, 2011-07-27 18:54 review
Messages (16)
msg138769 - (view) Author: py.user (py.user) * Date: 2011-06-21 03:50
>>> bytearray(b'abc').rjust(10, b'*')
bytearray(b'*******abc')
>>> bytearray(b'abc').rjust(10, bytearray(b'*'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: must be a byte string of length 1, not bytearray
>>>
msg138784 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-21 12:57
What's the use case?  I'm inclined to reject this as not needed.
msg138805 - (view) Author: py.user (py.user) * Date: 2011-06-21 22:18
all other methods support it and it's right

>>> barr = bytearray(b'abcd*')
>>> barr.center(len(barr) * 4, barr[-1:])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: must be a byte string of length 1, not bytearray
>>> b = b'abcd*'
>>> b.center(len(b) * 4, b[-1:])
b'*******abcd*********'
>>>
msg138807 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-21 22:51
A bytearray is for working with mutable data.  We don't support using it in all places that the non-mutable data types can be used.  You can code your example like this:

   barr.center(len(barr) * 4, bytes([barr[-1]]))

I realize that isn't particularly pretty, but that has more to do with the fact that indexing bytes gives you ints in Python 3 than it does with whether or not bytearray is accepted.

The data type of the arguments to the method have no necessary relationship with the datatype of the object.

You may have more success arguing that the fill character for both bytearray and bytes should be allowed to be an int.

I think this whole topic is better addressed in a forum such as python-ideas.  I agree that the bytes interface is a bit wonky in places, but I think that if changes are going to be made a consensus needs to be developed on what changes to make.  I believe some conversations about this have already taken place, and so far I don't think there are any consensus proposals.

So, I'm going to close this issue.  But please join (or start, if necessary) the discussion on this wider topic in the appropriate forum.
msg138809 - (view) Author: py.user (py.user) * Date: 2011-06-21 23:46
> A bytearray is for working with mutable data.  We don't support using > it in all places that the non-mutable data types can be used.

>>> bytearray(b'abcd').strip(bytearray(b'da'))
bytearray(b'bc')
>>>

.translate, .find, .partition, ...

>>> bytearray(b'.').join((bytearray(b'a'), bytearray(b'b')))
bytearray(b'a.b')
>>> bytearray(b'.').join((b'a', b'b'))
bytearray(b'a.b')
>>>

all these methods could use only bytes objects
msg138811 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-22 00:25
All right, let's get some other opinions from people who have actually worked with the bytearray and bytes code (and Terry because he cares about APIs).
msg138841 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-23 00:08
After thinking about this awhile, I see the key sentence of David's reply as "The data type of the arguments to the method have no necessary relationship with the datatype of the object." While true in general, in it not true with respect to corresponing text (string) and byte(array) methods. String parameters of strings methods become byte parameters of byte(array) methods. In the other hand, I think I agree with David's application to byte versus bytearray methods. I might change my mind after further examination of the methods in question. But for the present, I would not change the code.

Or would I? Here is a reason not to change. Example:

for byt in (b'abc', bytearray(b'cdef'), b'xye')
  yield byt.rjust(10,b'-')

Making the type of constant args depend on the type of the base object would make generic byte/bytearray functions more difficult. We already have this problem with writing functions that work with bytes and text in 3.x. It is a big nuisance that is only justified by the benefits of not mixing bytes and text. I do not think we should extend the nuisance to byte and bytearray functions, especially without a strong use case.

I marked this for 'documentation' because I think the doc for some of the str methods might be improved and that the reference to them in the bytes/bytearray definitely needs more. Doc changes would apply to 3.2 also.

"Bytes and bytearray objects, being “strings of bytes”, have all methods found on strings, with the exception ... " 

should be followed by something like. 

"If the string method has a string parameter, the corresponding byte/bytearray method has a corresponding byte parameter."

(to match the reported current behavior).

I have not yet looked at doc strings. I did not unmark 'Interpreter core' because I have not looked at all of p.u's examples to be sure that I like *all* of the current behaviors.
msg139373 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-06-28 18:47
I do agree it is a nuisance that it doesn't work with bytearray instances. After all, these methods are supposed to be homogeneous, and they are when called on a str or bytes object.
msg140495 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2011-07-16 14:08
On one hand, I agree that the situation isn't intuitive. Why should some methods of bytearray accept bytearrays, and some shouldn't?

On the other hand, this actually has rather deep implementation reasons. 

Methods like 'translate' are implemented in Objects/bytearrayobject.c

On the other hand, ljust, rjust and center are taken from stringlib. Now, stringlib is generic code, and has some strict argument checking. For example, in stringlib_ljust:

    if (!PyArg_ParseTuple(args, "n|c:ljust", &width, &fillchar))
        return NULL;

The 'c' format to PyArg_ParseTuple expects an object that passes PyBytes_Check, IOW a bytes object or a subclass thereof. bytearray is not a subclass of bytes, hence the problem.

The solution could be global, to allow bytearray fit the 'c' format of PyArg_ParseTuple. Then one would also be able to pass a bytearray into other stringlib methods requiring the 'c' format.

One way or the other, this is of course doable. A decision has to be made though - is the nuisance annoying enough to warrant such an API change?
msg141118 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-07-25 19:50
> The solution could be global, to allow bytearray fit the 'c' format of
> PyArg_ParseTuple. Then one would also be able to pass a bytearray into
> other stringlib methods requiring the 'c' format.

Another possibility would be the change the 'c' format so that it accepts any object that supports the buffer protocol and whose buffer length is 1.

Attaching two patches: The first allows bytes and bytearray, the second allows any object that supports the buffer protocol.
msg141128 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-07-25 23:10
c_format_bytearray.patch looks ok to me. The other proposal is too broad, and may lead to confusing behaviour.
In any case, some tests are needed.
msg141145 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-07-26 11:38
Updated the bytearray patch to change documentation and add tests.
msg141253 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2011-07-27 17:48
Looks good. How about also adding some tests for the original request of supporting bytearrays in ljust/rjust/center?
msg141264 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-07-27 18:54
Updated the patch to add tests for {bytes,bytearray}.{center,ljust,rjust}. The tests check that both bytes and bytearray are always accepted as the fill character.
msg141326 - (view) Author: Roundup Robot (python-dev) Date: 2011-07-29 04:07
New changeset 536fccc75f5a by Eli Bendersky in branch 'default':
Issue #12380: PyArg_ParseTuple now accepts a bytearray for the 'c' format.
http://hg.python.org/cpython/rev/536fccc75f5a
msg141327 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2011-07-29 04:08
Petri, thanks for the patch. I've updated Misc/NEWS and committed it.

Unless there are objections or problems, I will close this issue in a day or two.
History
Date User Action Args
2011-07-30 04:13:44eli.benderskysetstatus: open -> closed
2011-07-29 04:08:46eli.benderskysetresolution: fixed
messages: + msg141327
stage: patch review -> resolved
2011-07-29 04:07:06python-devsetnosy: + python-dev
messages: + msg141326
2011-07-27 18:54:20petri.lehtinensetfiles: + c_format_bytearray_plus_additional_tests.patch

messages: + msg141264
2011-07-27 17:48:35eli.benderskysetmessages: + msg141253
2011-07-26 11:38:25petri.lehtinensetfiles: + c_format_bytearray.patch

messages: + msg141145
2011-07-26 11:37:21petri.lehtinensetfiles: - c_format_buffer.patch
2011-07-26 11:37:12petri.lehtinensetfiles: - c_format_bytearray.patch
2011-07-25 23:10:05pitrousetmessages: + msg141128
2011-07-25 19:51:05petri.lehtinensetkeywords: + needs review
stage: needs patch -> patch review
2011-07-25 19:50:45petri.lehtinensetfiles: + c_format_buffer.patch
2011-07-25 19:50:14petri.lehtinensetfiles: + c_format_bytearray.patch
keywords: + patch
messages: + msg141118
2011-07-16 14:08:16eli.benderskysetmessages: + msg140495
2011-06-28 18:47:34pitrousetcomponents: - Documentation
2011-06-28 18:47:17pitrousetassignee: docs@python ->
messages: + msg139373
stage: resolved -> needs patch
2011-06-23 03:33:04eli.benderskysetnosy: + eli.bendersky
2011-06-23 00:08:33terry.reedysetnosy: + docs@python
messages: + msg138841

assignee: docs@python
components: + Documentation
resolution: rejected -> (no value)
2011-06-22 06:51:56petri.lehtinensetnosy: + petri.lehtinen
2011-06-22 00:25:38r.david.murraysetstatus: closed -> open
nosy: + rhettinger, terry.reedy, ncoghlan, pitrou, haypo
messages: + msg138811

2011-06-21 23:46:01py.usersetmessages: + msg138809
2011-06-21 22:51:47r.david.murraysetstatus: open -> closed
resolution: rejected
messages: + msg138807

stage: resolved
2011-06-21 22:18:54py.usersetmessages: + msg138805
2011-06-21 12:57:31r.david.murraysetversions: + Python 3.3, - Python 3.1, Python 2.7
nosy: + r.david.murray

messages: + msg138784

type: behavior -> enhancement
2011-06-21 03:51:00py.usersettitle: bytearray center, ljust, rjust don't accept a bytearray as the fill character -> bytearray methods center, ljust, rjust don't accept a bytearray as the fill character
2011-06-21 03:50:10py.usercreate