[issue5804] Add an 'offset' argument to zlib.decompress

[Martin Panter]

Martin Panter added the comment:

A different test case for “unused_data” attribute was added in 2012 for Issue 
16350, so that part is no longer needed.

If this feature goes ahead, it might be nice to also update the bzip and LZMA 
modules for consistency.

In Python 3, the equivalent of the buffer() option would look like this, 
assuming the memory view is in byte format:

zlib.decompress(memoryview(source)[unused_offset:])

Another option would be to copy some paint from the struct.unpack_from() 
bikeshed:

[data, offset] = zlib.decompress_from(buffer, offset)

--
nosy: +vadmium

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[R. David Murray]

R. David Murray added the comment:

Moving this from commit review back to no selection, since there doesn't yet 
seem to be an agreement on an API.

--
nosy: +r.david.murray
stage: commit review -> 
versions: +Python 3.5 -Python 3.2

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Mark Lawrence]

Changes by Mark Lawrence :


--
nosy:  -BreamoreBoy

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Nadeem Vawda]

Changes by Nadeem Vawda :


--
nosy: +nadeem.vawda

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Kristján Valur Jónsson]

Kristján Valur Jónsson  added the comment:

decompressobj is indeed "enough".  But if you are doing a lot of this 
(decompressing chunks), then using the unused_data, as it is, involves a lot of 
copying.
If there were a "unused_data_pos" or some equivalent, then it would be possible 
to continue decommpressing with buffer(olddata, unused_data_pos), without 
copying the source data.

The point of the "offset" keyword argument was to have a way to get this end 
position also without having an explicit decompressobj.

I agree that having the result type change to a tuple is not good.

So, a suggestion:
1) Add the unused_data_pos to the decompressobj.
2) (opotional) Add the automatic retrieval of this with a decompress_ex method, 
for convenience, making it usable without creating a decompressobj
3) (optional) Add the "offset" kw argument to decompress and (decompress_ex) 
making the creation of a temporary buffer() object unnecessary.

--

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc  added the comment:

The function returns different kind of data depending on the value of the last 
parameter. I don't like it at all.

Why is decompressobj not enough?

--
nosy: +amaury.forgeotdarc

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Mark Lawrence]

Mark Lawrence  added the comment:

This has been reviewed see msg87550, can we get this into 3.2?

--
nosy: +BreamoreBoy
stage: patch review -> commit review
versions: +Python 3.2 -Python 2.7, Python 3.1

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Robert Collins]

Robert Collins  added the comment:

Well, I think its relatively uncommon to be doing such a loop with a
static buffer anyway - often you'll instead be reading from disk or a
network stream; if we could make those cases simpler and avoid copying
that would be great.

Anyhow, no strong opinions here, just saw it going by before added my
2c.

-Rob

--

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Robert Collins]

Robert Collins  added the comment:

Maybe I'm missing something, but isn't the offset parameter just another
way of spelling
buffer(input, offset)?

I like the avoiding of copying, just wondering if having a magic
parameter to get a tuple is really better than (say)
result, used = zlib.decompress2(source)
if used

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Overall, this looks good.  Some mostly minor comments in this review.

http://codereview.appspot.com/63060/diff/1/2
File Doc/library/zlib.rst (right):

http://codereview.appspot.com/63060/diff/1/2#newcode136
Line 136: When specified, it will cause the function's return value to
be a (uncompressed, offset)
how about this wording:

..."to be a tuple of (uncompressed, offset), with the"...

http://codereview.appspot.com/63060/diff/1/2#newcode142
Line 142: Added the *offset* argument
missing . at the end of the sentence.  Also, mention that this function
now accepts keyword arguments.

http://codereview.appspot.com/63060/diff/1/3
File Lib/test/test_zlib.py (right):

http://codereview.appspot.com/63060/diff/1/3#newcode110
Line 110: c = zlib.compress(a)+zlib.compress(b)
please surround the + with spaces for readability and consistency with
other code in the file.\

http://codereview.appspot.com/63060/diff/1/4
File Modules/zlibmodule.c (right):

http://codereview.appspot.com/63060/diff/1/4#newcode193
Line 193: "the start position in the string to start decompresion and,
if spceified,\n"
typo: specified.

http://codereview.appspot.com/63060/diff/1/4#newcode296
Line 296: offset = length-zst.avail_in+offset;
leave spaces around your - and + operators and this becomes easier to
read.

http://codereview.appspot.com/63060

--

___
Python tracker 

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

[issue5804] Add an 'offset' argument to zlib.decompress

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
nosy: +gregory.p.smith

___
Python tracker 

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