[issue14684] zlib set dictionary support inflateSetDictionary
Jim Jewett jimjjew...@gmail.com added the comment: Just saw this on the checkins list; where are the other options documented? PyDoc_STRVAR(compressobj__doc__, -compressobj([level]) -- Return a compressor object.\n +compressobj([level[, method[, wbits[, memlevel[, strategy[, zdict]])\n + -- Return a compressor object.\n \n -Optional arg level is the compression level, in 1-9.); +Optional arg level is the compression level, in 1-9.\n +\n +Optional arg zdict is the predefined compression dictionary - a sequence of\n +bytes containing subsequences that are likely to occur in the input data.); I'm honestly not certain what they should be, but the following is my best guess: PyDoc_STRVAR(compressobj__doc__, compressobj([level[, method[, wbits[, memlevel[, strategy[, zdict]])\n -- Return a compressor object.\n \n -Optional arg level is the compression level, in 1-9.\n +Optional arg level (1-9) is the compression level.\n +Larger numbers take longer, but produce smaller results.\n \n +Optional arg method is the compression method.\n +The only currently supported method is zlib.DEFLATED.\n +\n +Optional arg wbits determines the window buffer size.\n +Normal values are 8 (least memory) to 15 (best compression).\n +\n +Optional arg memlevel (1-9) controls working memory size.\n +Larger numbers use more memory, but produce smaller results more quickly.\n +\n +Optional arg strategy tunes the compression algorithm.\n +Supported options include zlib.Z_DEFAULT_STRATEGY, zlib.Z_FILTERED, and zlib.Z_HUFFMAN_ONLY.\n +\n +Optional arg zdict is the predefined compression dictionary - a sequence of\n +bytes containing subsequences that are likely to occur in the input data.); -- nosy: +Jim.Jewett ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Nadeem Vawda nadeem.va...@gmail.com added the comment: Just saw this on the checkins list; where are the other options documented? They aren't, AFAIK. I've been planning on adding them when I've got time (based on the zlib manual at http://zlib.net/manual.html), but with the upcoming feature freeze for 3.3, this issue was higher priority. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 1cfa44cb5af0 by Nadeem Vawda in branch 'default': Document the rest of zlib.compressobj()'s arguments. http://hg.python.org/cpython/rev/1cfa44cb5af0 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset dd4f7d5c51c7 by Nadeem Vawda in branch 'default': Issue #14684: Add support for predefined compression dictionaries to the zlib module. http://hg.python.org/cpython/rev/dd4f7d5c51c7 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Nadeem Vawda nadeem.va...@gmail.com added the comment: Committed. Once again, thanks for the patch! -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Nadeem Vawda nadeem.va...@gmail.com added the comment: To restate my position: the need is for an immutable string of bytes, [...] I disagree that we should require the dictionary to be immutable - if the caller wishes to use a mutable buffer here, it is their responsibility to ensure that it is not modified until the compressor is finished with it (consenting adults and all that). The documentation can inform users of this requirement. I believe the argument for aesthetics does not apply, as the constant dictionary constructor argument is a morally different kind of parameter, comparable to (say) the compression level. Even so, the surrounding code sets a precedent for how it accepts binary data buffers, and deviating from this existing convention should not be taken lightly. Nitpicking about the API aside, thanks for the patch :-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Jesús Cea Avión j...@jcea.es added the comment: So my question is easy: could we apply this patch as is and defer any improvement to 3.4?. The risk of not doing so would be to miss 3.3 completely. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Nadeem Vawda nadeem.va...@gmail.com added the comment: I plan to commit it (along with the buffer API changes) tomorrow. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Jesús Cea Avión j...@jcea.es added the comment: Sam, the window for Python 3.3 integration is almost close. Could you possibly update your patch with Nadeem's feedback?. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: I think other than the disagreement about whether the dictionary constructor arg should be a buffer object, it's good to go. To restate my position: the need is for an immutable string of bytes, and that's exactly what PyBytes_Type is for. I see no advantage to allowing a buffer object, which will require extra code to check that it is both a buffer object and set to be readonly. I believe the argument for aesthetics does not apply, as the constant dictionary constructor argument is a morally different kind of parameter, comparable to (say) the compression level. You folks are of course welcome to change it, though. 8^) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Nadeem Vawda nadeem.va...@gmail.com added the comment: The code should be changed to use the buffer API (instead of accepting only bytes objects). Other than that, I think it's ready for integration. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Jesús Cea Avión j...@jcea.es added the comment: Status of this feature?. Ready to integrate? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Changes by Éric Araujo mer...@netwok.org: -- keywords: +needs review stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Changes by Nadeem Vawda nadeem.va...@gmail.com: -- nosy: +nadeem.vawda ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Nadeem Vawda nadeem.va...@gmail.com added the comment: I've posted a review on Rietveld. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: renames dict-zdict, splits the test, adds BEGIN/END around inflate call. -- Added file: http://bugs.python.org/file25472/zlib_set_dictionary_4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Jesús Cea Avión j...@jcea.es added the comment: A dictionary could be provided an init time. Then, the Z_NEED_DICT could be intercepted in the binding and automatically inject the dictionary provided in the init. Anyway, for a patch to be approved, we need a test too. PS: Why is this NOT targeted to 3.3?. We have time, yet. -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: I'm currently reworking this so that the dictionaries are provided in the constructor, and inflateSetDictionary() is called automatically. I've gone over the zlib RFC's and zlibmodule.c, and I'm fairly certain that whatever usage mode might involve multiple calls to SetDictionary() couldn't be supported by the zlib object anyway. r.e. 3.3/3.4 - I merely chose the highest version number I saw, since this diff is against HEAD (as recommended by the docs). It's been quite a few years since I've submitted a patch to CPython! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Jesús Cea Avión j...@jcea.es added the comment: Retargetting to python 3.3. If you hurry a bit and I find your patch acceptable (remember the tests!), I will try to integrate it. -- assignee: - serwy nosy: +serwy versions: +Python 3.3 -Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Changes by Jesús Cea Avión j...@jcea.es: -- assignee: serwy - nosy: -serwy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: Ok, here's the patch. It has a single short test. For use with SPDY, it's necessary to test that the following stream data also correctly decompresses, I'll attach that to the next comment. -- Added file: http://bugs.python.org/file25446/zlib_set_dictionary_2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: This test is rather large, since it includes the predefined SPDY draft 2 dictionary, and some real-world data. Not sure what the policy is on including so much data in a test. If there's enough time I could make a smaller test that also verifies the correct behavior on a stream... -- Added file: http://bugs.python.org/file25447/tz2.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: Argh, probably need to add the 'dict' field to the copy() method. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Sam Rushing rushing@gmail.com added the comment: Updated version of the patch: extends the test, including a test of the streaming behavior needed for SPDY (both compression and decompression). Also wik: copy()/uncopy() are aware of the 'dict' attribute. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Changes by Sam Rushing rushing@gmail.com: Added file: http://bugs.python.org/file25449/zlib_set_dictionary_3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
Éric Araujo mer...@netwok.org added the comment: Added a few comments on Rietveld. -- nosy: +eric.araujo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14684 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14684] zlib set dictionary support inflateSetDictionary
New submission from Sam Rushing rushing@gmail.com:
Google's SPDY protocol requires the use of a pre-defined compression
dictionary. The current zlib module doesn't expose the two functions for
setting the dictionary.
This patch is minimal in the sense that it only exposes the two functions, but
unfortunately the sequence of zlib calls required is clumsy: a call to
inflate() must fail first (with an error of Z_NEED_DICT):
import zlib
zdict = bthequickbrownfoxjumped\x00
c = zlib.compressobj()
c.set_dictionary (zdict)
cd = c.compress (bthe quick brown fox jumped over the candlestick)
cd += c.flush()
d = zlib.decompressobj()
try:
print (d.decompress (cd))
except zlib.error as what:
if what.args[0].startswith ('Error 2 '):
d.set_dictionary (zdict)
print (d.flush())
Obviously a better way to catch/match Z_NEED_DICT would be nice.
A much nicer solution would allow you to associate the dictionary at creation
time, rather than having to wait for an error condition. I can only guess that
the zlib authors designed it that way for a reason?
--
components: Extension Modules
files: zlib_set_dictionary.patch
keywords: patch
messages: 159492
nosy: Sam.Rushing
priority: normal
severity: normal
status: open
title: zlib set dictionary support inflateSetDictionary
type: enhancement
versions: Python 3.4
Added file: http://bugs.python.org/file25387/zlib_set_dictionary.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14684
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
