[issue16764] Make zlib accept keyword-arguments

[Martin Panter]

Changes by Martin Panter :


--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Xiang Zhang]

Xiang Zhang added the comment:

I think it's okay to close now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Martin Panter]

Martin Panter added the comment:

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?

--
stage: commit review -> resolved

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Serhiy, the message added in Misc/NEWS should be in Library section not Core 
> and Builtins section.

Done. And addressed Victor's reasonable comment. Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Roundup Robot]

Roundup Robot added the comment:

New changeset ab0039b8a80e 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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[STINNER Victor]

STINNER Victor added the comment:

Oops, I reviewed the patch before seeing that Serhiy already pushed it :-) 
Ignore my comment.

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Xiang Zhang]

Xiang Zhang added the comment:

Serhiy, the message added in Misc/NEWS should be in Library section not Core 
and Builtins section.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee: serhiy.storchaka -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Xiang Zhang]

Xiang Zhang added the comment:

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 for issue27574.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Roundup Robot]

Roundup Robot added the comment:

New changeset a4101218364e by Serhiy Storchaka in branch 'default':
Issue #16764: Support keyword arguments to zlib.decompress().  Patch by
https://hg.python.org/cpython/rev/a4101218364e

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

LGTM. With issue27574 the overhead is even smaller.

--
assignee:  -> serhiy.storchaka
stage: patch review -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Xiang Zhang]

Xiang Zhang added the comment:

OK. Simplest test with positional arguments.

Without patch:

./python -m timeit -s 'import zlib; a = 
zlib.compress(b"abcdefghijklmnopqrstuvwxyz")' 'zlib.decompress(a, 15, 16384)'
100 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)'
1 loops, best of 3: 16 usec per loop

With patch:

./python -m timeit -s 'import zlib; a = 
zlib.compress(b"abcdefghijklmnopqrstuvwxyz")' 'zlib.decompress(a, 15, 16384)'
100 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)'
1 loops, best of 3: 16.1 usec per loop

But, 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)'
100 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)'
1 loops, best of 3: 16.8 usec per loop

But with large data, the difference is gone:

./python -m timeit -s 'import zlib; a = 
zlib.compress(b"abcdefghijklmnopqrstuvwxyz"*1)' '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"*1)' 'zlib.decompress(a, 
wbits=15, bufsize=16384)'
1000 loops, best of 3: 252 usec per loop # with patch

So 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Martin Panter]

Martin Panter added the comment:

Xiang’s patch looks okay from a correctness point of view

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Xiang Zhang]

Xiang Zhang added the comment:

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.

--
Added file: http://bugs.python.org/file44074/zlib_keyword_argument.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Xiang Zhang]

Changes by Xiang Zhang :


--
nosy: +xiang.zhang

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--
versions: +Python 3.6 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Stéphane Wirtel]

Stéphane Wirtel added the comment:

martin or serhiy,

can we move this issue to 3.6 ?

--
nosy: +matrixise

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Martin Panter]

Martin Panter added the comment:

See Issue 8706 discussing adding keyword support to library functions in 
general.

Functions the first patch affects:

* zlib.compress(data, level=...): still relevant; see Issue 26243
* compressobj(memlevel=...): fixed in documentation in revision fdb5d84f9948 
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; see 
Issue 23200

Most of the pep8 patch looks like pointless noise, and a lot of the remaining 
bits have probably already been addressed.

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Lukas Lueg]

New submission from Lukas Lueg:

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.

--
components: Library (Lib)
files: zlib_keywords.patch
keywords: patch
messages: 178053
nosy: ebfe
priority: normal
severity: normal
status: open
title: Make zlib accept keyword-arguments
versions: Python 3.4
Added file: http://bugs.python.org/file28418/zlib_keywords.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16764
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Lukas Lueg]

Lukas Lueg added the comment:

Attaching a patch to fix all pep8/pyflakes warnings and errors in test_zlib.py

--
Added file: http://bugs.python.org/file28419/zlib_tests_pep8.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16764
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
nosy: +ezio.melotti

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16764
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--
components: +Extension Modules -Library (Lib)
nosy: +serhiy.storchaka
stage:  - patch review
type:  - enhancement

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16764
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Lukas Lueg]

Lukas Lueg added the comment:

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.

--
components: +Library (Lib) -Extension Modules
type: enhancement - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16764
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16764] Make zlib accept keyword-arguments

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Sorry, I missed, not decompress(), but compress(). Except this small error all 
of what I said *is* a problem of this patch.

--
components: +Extension Modules -Library (Lib)
type:  - enhancement

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16764
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com