Movatterモバイル変換


[0]ホーム

URL:


homepage

Issue16764

This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title:Make zlib accept keyword-arguments
Type:enhancementStage:resolved
Components:Extension ModulesVersions:Python 3.6
process
Status:closedResolution:fixed
Dependencies:Superseder:
Assigned To:Nosy List: ebfe, ezio.melotti, martin.panter, matrixise, python-dev, serhiy.storchaka, vstinner, xiang.zhang
Priority:normalKeywords:patch

Created on2012-12-24 12:45 byebfe, last changed2022-04-11 14:57 byadmin. This issue is nowclosed.

Files
File nameUploadedDescriptionEdit
zlib_keywords.patchebfe,2012-12-24 12:45review
zlib_tests_pep8.patchebfe,2012-12-24 12:46PEP8/PyFlakes-fixes for test_zlib.py
zlib_keyword_argument.patchxiang.zhang,2016-08-11 06:58review
Messages (21)
msg178053 -(view)Author: Lukas Lueg (ebfe)Date: 2012-12-24 12:45
The patch "zlib_keywords.patch" makes zlib's classes and functions accept keyword arguments as documented. It also fixes two cases in which the docstring differ from the documentation (decompress(data) vs. decompress(string) and compressobj(memlevel) vs. compressobj(memLevel)). Additional tests are provided.
msg178054 -(view)Author: Lukas Lueg (ebfe)Date: 2012-12-24 12:46
Attaching a patch to fix all pep8/pyflakes warnings and errors in test_zlib.py
msg178058 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2012-12-24 13:39
I don't think it worth to add support of keyword for the first mandatory argument. This can freeze the poor and inconsistent () names. For example compress()/decompress() methods of bz2 and lzma objects doesn't support keyword arguments. And why you use "string" name for decompress() argument?Renaming of "memLevel" argument to "memlevel" is not backward compatible and can break third-part code (if anyone use it). This may require starting of deprecation process. Difference between a code and a documentation is a bug and should be fixed for all Python versions.Note, that calling a function with keyword arguments is a little slower (a lot of slower for fast functions) than calling a function with positional-only arguments.
msg178059 -(view)Author: Lukas Lueg (ebfe)Date: 2012-12-24 14:31
Nothing of what you mention is a problem of this patch.The memLevel-keyword was not supported as of now, only the docstring ("memLevel") and the documentation ("memlevel") mentioned it. There is no third-party code that could have used it.The current docstring says that a "string"-keyword should be used with decompress(), the documentation talks about a "data"-keyword. Both are not supported, the patch adds support for a "data"-keyword and fixes the docstring.
msg178063 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2012-12-24 14:58
Sorry, I missed, not decompress(), but compress(). Except this small error all of what I said *is* a problem of this patch.
msg259453 -(view)Author: Martin Panter (martin.panter)*(Python committer)Date: 2016-02-03 03:00
SeeIssue 8706 discussing adding keyword support to library functions in general.Functions the first patch affects:* zlib.compress(data, level=...): still relevant; seeIssue 26243* compressobj(memlevel=...): fixed in documentation in revisionfdb5d84f9948 instead* compressobj.compress(data): still relevant, but little benefit IMO* compressobj.flush(mode=...): still relevant; little benefit* zlib.decompress(data, wbits=..., bufsize=...): still relevant* decompressobj.decompress(data, max_length=...): still relevant* decompressobj.flush(length=...): still relevant, but not worthwhile IMO; seeIssue 23200Most of thepep8 patch looks like pointless noise, and a lot of the remaining bits have probably already been addressed.
msg272350 -(view)Author: Stéphane Wirtel (matrixise)*(Python committer)Date: 2016-08-10 17:32
martin or serhiy,can we move this issue to 3.6 ?
msg272357 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2016-08-10 18:19
PyArg_ParseTupleAndKeywords() and Argument Clinic now support positional-only arguments. Thus this my objection can be resolved. But performance argument still needs an investigation.In any case the patch needs updating, since the zlib module was rewritten for using Argument Clinic.
msg272404 -(view)Author: Xiang Zhang (xiang.zhang)*(Python committer)Date: 2016-08-11 06:58
I agree with Martin and suggest make the following changes:1. Make wbits and bufsize of zlib.decompress keyword arguments.2. Make max_length of decompressobj.decompress keyword argument.I'd prefer others to stay position-only as now.Attach a patch doing the above 2 changes.
msg272437 -(view)Author: Martin Panter (martin.panter)*(Python committer)Date: 2016-08-11 11:01
Xiang’s patch looks okay from a correctness point of view
msg272481 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2016-08-11 20:23
Xiang Zhang, could you please provide results of benchmarking zlib.decompress and decompressobj.decompress in simplest case with and without the patch? PyArg_ParseTupleAndKeywords can be slower than PyArg_ParseTuple even for positional arguments, and we should know how much.
msg272491 -(view)Author: Xiang Zhang (xiang.zhang)*(Python committer)Date: 2016-08-12 03:14
OK. Simplest test with positional arguments.Without patch:./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz")' 'zlib.decompress(a, 15, 16384)'1000000 loops, best of 3: 0.841 usec per loop./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz"); do = zlib.decompressobj()' 'do.decompress(a, 100)'10000 loops, best of 3: 16 usec per loopWith patch:./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz")' 'zlib.decompress(a, 15, 16384)'1000000 loops, best of 3: 0.843 usec per loop./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz"); do = zlib.decompressobj()' 'do.decompress(a, 100)'10000 loops, best of 3: 16.1 usec per loopBut, with keyword specified, there is a degrade../python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz")' 'zlib.decompress(a, wbits=15, bufsize=16384)'1000000 loops, best of 3: 1.26 usec per loop./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz"); do = zlib.decompressobj()' 'do.decompress(a, max_length=100)'10000 loops, best of 3: 16.8 usec per loopBut with large data, the difference is gone:./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz"*10000)' 'zlib.decompress(a, 15, 16384)'1000 loops, best of 3: 252 usec per loop # without patch ./python -m timeit -s 'import zlib; a = zlib.compress(b"abcdefghijklmnopqrstuvwxyz"*10000)' 'zlib.decompress(a, wbits=15, bufsize=16384)'1000 loops, best of 3: 252 usec per loop # with patchSo I think it's OK for this change. There seems no performance degrade to old code. And considering that zlib usually does time consuming tasks (I don't think it's common to decompress such small data), the small slower down seems affordable.
msg272722 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2016-08-15 06:58
LGTM. Withissue27574 the overhead is even smaller.
msg272723 -(view)Author: Roundup Robot (python-dev)(Python triager)Date: 2016-08-15 07:06
New changeseta4101218364e by Serhiy Storchaka in branch 'default':Issue#16764: Support keyword arguments to zlib.decompress().  Patch byhttps://hg.python.org/cpython/rev/a4101218364e
msg272731 -(view)Author: Xiang Zhang (xiang.zhang)*(Python committer)Date: 2016-08-15 07:47
Oops! I am on the way regenerating the CA output to catch up with hg tip, but after a meeting you have done it.Thanks for your work, Serhiy. And excellent job as forissue27574.
msg272897 -(view)Author: Xiang Zhang (xiang.zhang)*(Python committer)Date: 2016-08-17 03:03
Serhiy, the message added inMisc/NEWS should be in Library section not Core and Builtins section.
msg272920 -(view)Author: STINNER Victor (vstinner)*(Python committer)Date: 2016-08-17 11:48
Oops, I reviewed the patch before seeing that Serhiy already pushed it :-) Ignore my comment.
msg273014 -(view)Author: Roundup Robot (python-dev)(Python triager)Date: 2016-08-18 06:15
New changesetab0039b8a80e by Serhiy Storchaka in branch 'default':Issue#16764: Move NEWS entry to correct section and remove too strict test.https://hg.python.org/cpython/rev/ab0039b8a80e
msg273015 -(view)Author: Serhiy Storchaka (serhiy.storchaka)*(Python committer)Date: 2016-08-18 06:17
> Serhiy, the message added inMisc/NEWS should be in Library section not Core and Builtins section.Done. And addressed Victor's reasonable comment. Thanks!
msg273271 -(view)Author: Martin Panter (martin.panter)*(Python committer)Date: 2016-08-21 06:42
All the interesting keyword arguments seem to work now (checking against my notes from earlier). Is there anything else anyone wants to do, or can we close this now?
msg273279 -(view)Author: Xiang Zhang (xiang.zhang)*(Python committer)Date: 2016-08-21 08:19
I think it's okay to close now.
History
DateUserActionArgs
2022-04-11 14:57:39adminsetgithub: 60968
2016-09-04 04:17:30martin.pantersetstatus: open -> closed
resolution: fixed
2016-08-21 08:19:24xiang.zhangsetmessages: +msg273279
2016-08-21 06:42:22martin.pantersetmessages: +msg273271
stage: commit review -> resolved
2016-08-18 06:17:18serhiy.storchakasetmessages: +msg273015
2016-08-18 06:15:23python-devsetmessages: +msg273014
2016-08-17 11:48:50vstinnersetnosy: +vstinner
messages: +msg272920
2016-08-17 03:03:55xiang.zhangsetmessages: +msg272897
2016-08-15 08:55:00serhiy.storchakasetassignee:serhiy.storchaka ->
2016-08-15 07:47:22xiang.zhangsetmessages: +msg272731
2016-08-15 07:06:34python-devsetnosy: +python-dev
messages: +msg272723
2016-08-15 06:58:49serhiy.storchakasetassignee:serhiy.storchaka
messages: +msg272722
stage: patch review -> commit review
2016-08-12 03:14:18xiang.zhangsetmessages: +msg272491
2016-08-11 20:23:56serhiy.storchakasetmessages: +msg272481
2016-08-11 11:01:36martin.pantersetmessages: +msg272437
2016-08-11 06:58:28xiang.zhangsetfiles: +zlib_keyword_argument.patch

messages: +msg272404
2016-08-11 04:11:52xiang.zhangsetnosy: +xiang.zhang
2016-08-10 18:19:02serhiy.storchakasetmessages: +msg272357
versions: + Python 3.6, - Python 3.4
2016-08-10 17:32:55matrixisesetnosy: +matrixise
messages: +msg272350
2016-02-03 03:00:18martin.pantersetnosy: +martin.panter
messages: +msg259453
2012-12-24 14:58:05serhiy.storchakasettype: enhancement
messages: +msg178063
components: + Extension Modules, - Library (Lib)
2012-12-24 14:31:45ebfesettype: enhancement -> (no value)
messages: +msg178059
components: + Library (Lib), - Extension Modules
2012-12-24 13:39:05serhiy.storchakasetnosy: +serhiy.storchaka
messages: +msg178058

components: + Extension Modules, - Library (Lib)
type: enhancement
stage: patch review
2012-12-24 12:54:10ezio.melottisetnosy: +ezio.melotti
2012-12-24 12:46:58ebfesetfiles: +zlib_tests_pep8.patch

messages: +msg178054
2012-12-24 12:45:08ebfecreate
Supported byThe Python Software Foundation,
Powered byRoundup
Copyright © 1990-2022,Python Software Foundation
Legal Statements

[8]ページ先頭

©2009-2026 Movatter.jp