Add an optional newline parameter to binascii.b2a_base64() to not add a newline

[vstinner]

BPO	25357
Nosy	@ncoghlan, @pitrou, @vstinner, @vadmium, @serhiy-storchaka
Files	binascii_b2a_base64_newline.patch binascii_b2a_base64_newline-2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-10-11.09:02:38.115>
created_at = <Date 2015-10-09.16:21:56.846>
labels = ['performance']
title = 'Add an optional newline parameter to binascii.b2a_base64() to not add a newline'
updated_at = <Date 2015-10-11.09:02:38.114>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-10-11.09:02:38.114>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-10-11.09:02:38.115>
closer = 'vstinner'
components = []
creation = <Date 2015-10-09.16:21:56.846>
creator = 'vstinner'
dependencies = []
files = ['40731', '40733']
hgrepos = []
issue_num = 25357
keywords = ['patch']
message_count = 9.0
messages = ['252625', '252627', '252630', '252631', '252632', '252684', '252692', '252791', '252792']
nosy_count = 6.0
nosy_names = ['ncoghlan', 'pitrou', 'vstinner', 'python-dev', 'martin.panter', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue25357'
versions = ['Python 3.6']

[vstinner]

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.

[serhiy-storchaka]

+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()?

[vstinner]

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.

[vstinner]

Updated patch to take Serhiy's comments in account and now with an unit test!

[vstinner]

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.

[vadmium]

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.

[serhiy-storchaka]

With Martin's comments the patch LGTM.

[python-dev]

New changeset 463a09a3bfff by Victor Stinner in branch 'default':
Issue bpo-25357: Add an optional newline paramer to binascii.b2a_base64().
https://hg.python.org/cpython/rev/463a09a3bfff

[vstinner]

Thanks for reviews Serhiy & Martin.