➜
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2015-10-09 16:21 by vstinner, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| binascii_b2a_base64_newline.patch | vstinner, 2015-10-09 16:21 | review | ||
| binascii_b2a_base64_newline-2.patch | vstinner, 2015-10-09 17:05 | review | ||
| Messages (9) | |||
|---|---|---|---|
| msg252625 - (view) | Author: STINNER Victor (vstinner) *  |
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) * |
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 (vstinner) * |
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 (vstinner) * |
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 (vstinner) * |
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) * |
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) * |
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 (vstinner) * |
Date: 2015-10-11 09:02 | |
Thanks for reviews Serhiy & Martin. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:22 | admin | set | github: 69544 |
| 2015-10-11 09:02:38 | vstinner | set | status: open -> closed resolution: fixed messages: + msg252792 |
| 2015-10-11 09:01:35 | python-dev | set | nosy:
+ python-dev messages: + msg252791 |
| 2015-10-10 06:24:04 | serhiy.storchaka | set | messages: + msg252692 |
| 2015-10-10 04:01:41 | martin.panter | set | nosy:
+ martin.panter messages: + msg252684 |
| 2015-10-09 17:07:58 | vstinner | set | messages: + msg252632 |
| 2015-10-09 17:05:30 | vstinner | set | files:
+ binascii_b2a_base64_newline-2.patch messages: + msg252631 |
| 2015-10-09 16:58:58 | vstinner | set | messages: + msg252630 |
| 2015-10-09 16:45:09 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka, ncoghlan, pitrou messages: + msg252627 |
| 2015-10-09 16:21:56 | vstinner | create | |