Repository navigation
zlib.compress level as keyword argument #70431
Description
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 2016-06-25.19:48:19.446>
created_at = <Date 2016-01-30.18:41:07.895>
labels = ['extension-modules', 'type-feature']
title = 'zlib.compress level as keyword argument'
updated_at = <Date 2019-02-04.13:45:18.673>
user = 'https://github.com/palaviv'bugs.python.org fields:
activity = <Date 2019-02-04.13:45:18.673>
actor = 'lukasz.langa'
assignee = 'none'
closed = True
closed_date = <Date 2016-06-25.19:48:19.446>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2016-01-30.18:41:07.895>
creator = 'palaviv'
dependencies = []
files = ['41760', '41773', '41797', '43320', '43334']
hgrepos = []
issue_num = 26243
keywords = ['patch']
message_count = 16.0
messages = ['259266', '259270', '259332', '259358', '259388', '259452', '259498', '259921', '259925', '259926', '268025', '268144', '269098', '269249', '269250', '269251']
nosy_count = 7.0
nosy_names = ['twouters', 'nadeem.vawda', 'python-dev', 'berker.peksag', 'martin.panter', 'serhiy.storchaka', 'palaviv']
pr_nums = ['11754', '11754', '11754']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26243'
versions = ['Python 3.6']
Activity
palaviv commentedon Jan 30, 2016
Currently zlib.compress allow only positional arguments. For code readability reasons i think that we should allow the level argument to be keyword argument. Now when someone uses zlib.compress he will be able to do zlib.compess(some_data, level=7) instead of zlib.compress(some_data, 7).
There is a patch included with the change.
vadmium commentedon Jan 30, 2016
Seems like a reasonable enhancement. A couple of points:
palaviv commentedon Feb 1, 2016
Thanks for the review.
vadmium commentedon Feb 2, 2016
Thanks, it looks good to me. I think “data” is a better name than “bytes”.
About What’s New, I thought the general idea was to document all enhancements, though I admit this is about as small as they come. There is precedent with the compile() builtin (bpo-1444529, Python 2.6), although there is no indication of kwargs in the main documentation for compile(). But on the other hand, str.split() didn’t get any What’s New entry (bpo-14081, Python 3.3). See also <https://docs.python.org/devguide/committing.html#what-s-new-and-news-entries\>.
berkerpeksag commentedon Feb 2, 2016
There is an outdated patch in bpo-16764 for zlib.compress(), zlib.decompress(), zlib.flush() and other functions. Perhaps we can close it as a duplicate of this one.
vadmium commentedon Feb 3, 2016
The patch at bpo-16764 has a test case for the new level=... keyword. Perhaps you would like to incorporate/update it :)
palaviv commentedon Feb 3, 2016
Added the "suggested" test
python-dev commentedon Feb 9, 2016
New changeset 592db8704d38 by Martin Panter in branch 'default':
Issue bpo-26243: zlib.compress() keyword argument support by Aviv Palivoda
https://hg.python.org/cpython/rev/592db8704d38
python-dev commentedon Feb 9, 2016
New changeset dc758f51b8f5 by Martin Panter in branch 'default':
bpo-26243: Forgot to update zlib doc strings in Argument Clinic
https://hg.python.org/cpython/rev/dc758f51b8f5
vadmium commentedon Feb 9, 2016
I did add a What’s New entry (I hope nobody minds), and updated the doc strings in the zlib module. Thanks for the work on this Aviv.
serhiy-storchaka commentedon Jun 9, 2016
The original request was for supporting level as keyword argument. Making the first argument a keyword argument was unintentional side effect (due a to the limitation of argument parsing functions). Now it is possible to support positional-only and keyword arguments in one function (see bpo-26282). Proposed patch makes the first argument positional-only again.
serhiy-storchaka commentedon Jun 10, 2016
Regenerated for review.
vadmium commentedon Jun 23, 2016
The patch (with Berker’s fix) looks okay. Personally, I don’t see a big problem with the first argument also having a keyword name, but I don’t mind if it doesn’t either.
python-dev commentedon Jun 25, 2016
New changeset 01f82a7a1250 by Serhiy Storchaka in branch 'default':
Issue bpo-26243: Only the level argument to zlib.compress() is keyword argument
https://hg.python.org/cpython/rev/01f82a7a1250
python-dev commentedon Jun 25, 2016
New changeset ab2e7e51869d by Serhiy Storchaka in branch 'default':
Issue bpo-26243: Correct a wording in docs.
https://hg.python.org/cpython/rev/ab2e7e51869d
serhiy-storchaka commentedon Jun 25, 2016
Thanks Berker and Martin.