classification
Title: Add an optional newline parameter to binascii.b2a_base64() to not add a newline
Type: performance Stage:
Components: Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: haypo, martin.panter, ncoghlan, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-10-09 16:21 by haypo, last changed 2015-10-11 09:02 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
binascii_b2a_base64_newline.patch haypo, 2015-10-09 16:21 review
binascii_b2a_base64_newline-2.patch haypo, 2015-10-09 17:05 review
Messages (9)
msg252625 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-09 16:21
The base64.b64encode() function calls binascii.b2a_base64() and then strips the newline. It would be more efficient to directly not add a newline.

Attached patch adds an optional newline parameter to binascii.b2a_base64(). It also modifies base64.b64encode() to call binascii.b2a_base64() with newline=False.
msg252627 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-09 16:45
+1. Not sure about the parameter name, but if no one suggests better, it LGTM. Added comments on Rietveld.

What about adding the same parameter to binascii.b2a_uu()?
msg252630 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-09 16:58
Oh, I forgot to add an unit test.

> What about adding the same parameter to binascii.b2a_uu()?

This function is used in Lib/encodings/uu_codec.py and Lib/uu.py, but the newline is not stripped. So I don't think that it's worth to add an optional parameter.
msg252631 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-09 17:05
Updated patch to take Serhiy's comments in account and now with an unit test!
msg252632 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-09 17:07
Oh by the way, the new parameter is a keyword only parameter. It's not possible to write b2a_base64(data, False). Maybe I should also add an unit test for this.
msg252684 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-10 04:01
The “newline” name seems as good as any. The patch looks good in general. I left a few comments.

I agree with not bothering for the UU encoder. That encoding has an embedded line length byte at the start of each line, so it makes less sense to omit newlines. Base-64 is used in places such as URLs where multiline output is not relevant, but I don’t know of a similar case for the UU encoding.

If I was doing it, I wouldn’t bother with a test case for keyword-only-ness, but suit yourself.
msg252692 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-10 06:24
With Martin's comments the patch LGTM.
msg252791 - (view) Author: Roundup Robot (python-dev) Date: 2015-10-11 09:01
New changeset 463a09a3bfff by Victor Stinner in branch 'default':
Issue #25357: Add an optional newline paramer to binascii.b2a_base64().
https://hg.python.org/cpython/rev/463a09a3bfff
msg252792 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-11 09:02
Thanks for reviews Serhiy & Martin.
History
Date User Action Args
2015-10-11 09:02:38hayposetstatus: open -> closed
resolution: fixed
messages: + msg252792
2015-10-11 09:01:35python-devsetnosy: + python-dev
messages: + msg252791
2015-10-10 06:24:04serhiy.storchakasetmessages: + msg252692
2015-10-10 04:01:41martin.pantersetnosy: + martin.panter
messages: + msg252684
2015-10-09 17:07:58hayposetmessages: + msg252632
2015-10-09 17:05:30hayposetfiles: + binascii_b2a_base64_newline-2.patch

messages: + msg252631
2015-10-09 16:58:58hayposetmessages: + msg252630
2015-10-09 16:45:09serhiy.storchakasetnosy: + serhiy.storchaka, ncoghlan, pitrou
messages: + msg252627
2015-10-09 16:21:56haypocreate