zlib.compress level as keyword argument

[palaviv]

BPO	26243
Nosy	@Yhg1s, @berkerpeksag, @vadmium, @serhiy-storchaka, @palaviv
PRs	bpo-26243: data= is positional-only just on CPython #11754 bpo-26243: data= is positional-only just on CPython #11754 bpo-26243: data= is positional-only just on CPython #11754
Files	zlib-compress-level-keyword.patch zlib-compress-level-keyword2.patch zlib-compress-level-keyword3.patch: patch including test zlib_compress_positional_only_data.patch zlib_compress_positional_only_data.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 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']

[palaviv]

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]

Seems like a reasonable enhancement. A couple of points:

• Watch out for the parameter names. Should it be “bytes” or “data”?
• Z_DEFAULT_COMPRESSION is -1 not 6 (see bpo-26244)
• Needs a version changed notice, and probably a What’s New update

[palaviv]

Thanks for the review.

1. Changed "bytes" to "data".
2. Updated Doc as in bpo-26244 (will update the patch if there will be changes there).
3. Added version changed notice but i don't think there is a need for What's new update.

[vadmium]

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]

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]

The patch at bpo-16764 has a test case for the new level=... keyword. Perhaps you would like to incorporate/update it :)

[palaviv]

Added the "suggested" test

[python-dev]

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]

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]

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.