classification
Title: zlib.compress level as keyword argument
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, martin.panter, nadeem.vawda, palaviv, python-dev, serhiy.storchaka, twouters
Priority: normal Keywords: patch

Created on 2016-01-30 18:41 by palaviv, last changed 2016-06-25 19:48 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
zlib-compress-level-keyword.patch palaviv, 2016-01-30 18:41 review
zlib-compress-level-keyword2.patch palaviv, 2016-02-01 17:06 review
zlib-compress-level-keyword3.patch palaviv, 2016-02-03 17:48 patch including test review
zlib_compress_positional_only_data.patch serhiy.storchaka, 2016-06-09 13:42
zlib_compress_positional_only_data.patch serhiy.storchaka, 2016-06-10 18:57 review
Messages (16)
msg259266 - (view) Author: Aviv Palivoda (palaviv) * Date: 2016-01-30 18:41
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.
msg259270 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-30 21:35
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 Issue 26244)
* Needs a version changed notice, and probably a What’s New update
msg259332 - (view) Author: Aviv Palivoda (palaviv) * Date: 2016-02-01 17:06
Thanks for the review.
1. Changed "bytes" to "data".
2. Updated Doc as in issue 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.
msg259358 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-02 05:12
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 (Issue 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 (Issue 14081, Python 3.3). See also <https://docs.python.org/devguide/committing.html#what-s-new-and-news-entries>.
msg259388 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-02-02 14:13
There is an outdated patch in issue 16764 for zlib.compress(), zlib.decompress(), zlib.flush() and other functions. Perhaps we can close it as a duplicate of this one.
msg259452 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-03 02:59
The patch at Issue 16764 has a test case for the new level=... keyword. Perhaps you would like to incorporate/update it :)
msg259498 - (view) Author: Aviv Palivoda (palaviv) * Date: 2016-02-03 17:48
Added the "suggested" test
msg259921 - (view) Author: Roundup Robot (python-dev) Date: 2016-02-09 10:24
New changeset 592db8704d38 by Martin Panter in branch 'default':
Issue #26243: zlib.compress() keyword argument support by Aviv Palivoda
https://hg.python.org/cpython/rev/592db8704d38
msg259925 - (view) Author: Roundup Robot (python-dev) Date: 2016-02-09 10:46
New changeset dc758f51b8f5 by Martin Panter in branch 'default':
Issue 26243: Forgot to update zlib doc strings in Argument Clinic
https://hg.python.org/cpython/rev/dc758f51b8f5
msg259926 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-09 10:46
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.
msg268025 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-09 13:42
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 issue26282). Proposed patch makes the first argument positional-only again.
msg268144 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-10 18:57
Regenerated for review.
msg269098 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-23 01:09
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.
msg269249 - (view) Author: Roundup Robot (python-dev) Date: 2016-06-25 19:43
New changeset 01f82a7a1250 by Serhiy Storchaka in branch 'default':
Issue #26243: Only the level argument to zlib.compress() is keyword argument
https://hg.python.org/cpython/rev/01f82a7a1250
msg269250 - (view) Author: Roundup Robot (python-dev) Date: 2016-06-25 19:47
New changeset ab2e7e51869d by Serhiy Storchaka in branch 'default':
Issue #26243: Correct a wording in docs.
https://hg.python.org/cpython/rev/ab2e7e51869d
msg269251 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-25 19:48
Thanks Berker and Martin.
History
Date User Action Args
2016-06-25 19:48:19serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg269251

stage: patch review -> resolved
2016-06-25 19:47:27python-devsetmessages: + msg269250
2016-06-25 19:43:24python-devsetmessages: + msg269249
2016-06-23 01:09:08martin.pantersetmessages: + msg269098
2016-06-10 18:57:15serhiy.storchakasetfiles: + zlib_compress_positional_only_data.patch

messages: + msg268144
2016-06-09 13:42:32serhiy.storchakasetstatus: closed -> open
files: + zlib_compress_positional_only_data.patch


nosy: + serhiy.storchaka
messages: + msg268025
resolution: fixed -> (no value)
stage: resolved -> patch review
2016-02-09 10:46:52martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg259926

stage: patch review -> resolved
2016-02-09 10:46:17python-devsetmessages: + msg259925
2016-02-09 10:24:19python-devsetnosy: + python-dev
messages: + msg259921
2016-02-03 17:48:17palavivsetfiles: + zlib-compress-level-keyword3.patch

messages: + msg259498
2016-02-03 02:59:20martin.pantersetmessages: + msg259452
2016-02-02 14:13:45berker.peksagsetnosy: + berker.peksag
messages: + msg259388
2016-02-02 05:12:10martin.pantersetmessages: + msg259358
2016-02-01 17:06:36palavivsetfiles: + zlib-compress-level-keyword2.patch

messages: + msg259332
2016-01-30 21:35:13martin.pantersetnosy: + martin.panter
messages: + msg259270

type: behavior -> enhancement
stage: patch review
2016-01-30 18:41:07palavivcreate