classification
Title: Add {encode, decode}_filter_properties() functions to lzma module
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: nadeem.vawda, pitrou, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-05-06 01:08 by nadeem.vawda, last changed 2012-05-07 08:47 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
lzma-properties.diff nadeem.vawda, 2012-05-06 01:08 review
lzma-properties.diff nadeem.vawda, 2012-05-06 10:34 Updated patch - tweaked docs and comments, and added tests. review
Messages (8)
msg160051 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-05-06 01:08
Patch attached. Reviews welcome.
msg160063 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-06 08:27
The functionality looks a bit cryptic to me. What is the use case?

I wonder if Py_LONG_LONG is always defined (although it certainly is on major platforms).
Other than that, the patch looks technically correct, though I'm not an lzma expert.
msg160071 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-05-06 10:34
> The functionality looks a bit cryptic to me. What is the use case?

Serializing filter specifiers for custom file formats. The particular
case that prompted adding the code is zipfile (issue 14366).

I've added a note to the docs and docstrings explaining this.


> I wonder if Py_LONG_LONG is always defined (although it certainly is on major platforms).

I expect it will always be defined on platforms that support liblzma -
the API uses uint64_t in quite a few places, via the lzma_vli typedef.

In any case, _lzmamodule.c checks for PY_LONG_LONG explicitly at
compile-time and gives a useful error message if it is undefined.
msg160073 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-06 10:38
The updated patch looks ok to me.
msg160108 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-05-06 21:10
Committed as changeset 9118ef2b651a.
msg160115 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-05-06 23:05
Thank you, Nadeem Vawda. I also wrote a patch for this, but because of the lack of experience it was too cumbersome.

But there are no tests for these functions. I tried to use these functions and got the random values.

>>> lzma.decode_filter_properties(lzma.FILTER_LZMA1, lzma.encode_filter_properties({'id': lzma.FILTER_LZMA1, 'preset': 6}))
{'mode': 3075124424, 'dict_size': 8388608, 'id': 4611686018427387905, 'lp': 0, 'nice_len': 739426833, 'depth': 0, 'lc': 3, 'mf': 0, 'pb': 2}
>>> lzma.decode_filter_properties(lzma.FILTER_LZMA1, lzma.encode_filter_properties({'id': lzma.FILTER_LZMA1, 'preset': 6}))
{'mode': 0, 'dict_size': 8388608, 'id': 4611686018427387905, 'lp': 0, 'nice_len': 2053908595, 'depth': 0, 'lc': 3, 'mf': 0, 'pb': 2}
>>> lzma.decode_filter_properties(lzma.FILTER_LZMA1, lzma.encode_filter_properties({'id': lzma.FILTER_LZMA1, 'preset': 6}))
{'mode': 0, 'dict_size': 8388608, 'id': 4611686018427387905, 'lp': 0, 'nice_len': 2053908595, 'depth': 0, 'lc': 3, 'mf': 0, 'pb': 2}

It seems, 'mode' and 'nice_len' was not initialized.

Tests for zipfile module with the use of these functions are broken, and I don't know, this happens because of errors in these functions or in zipfile module.
msg160118 - (view) Author: Nadeem Vawda (nadeem.vawda) * (Python committer) Date: 2012-05-07 05:11
Changeset 9118ef2b651a was broken, but the bug should have been fixed by
changeset 10ccbb90a8e9. Which revision have you been using?

> But there are no tests for these functions.

There *are* tests for these functions, and they were failing on some of
the buildbots for the original changeset. They are currently passing.
msg160122 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-05-07 08:47
> Changeset 9118ef2b651a was broken, but the bug should have been fixed by
> changeset 10ccbb90a8e9. Which revision have you been using?

I used the revision 76809:ab57e29157bb. Yes, now the bug fixed. Thank
you once again.
History
Date User Action Args
2012-05-07 08:47:50serhiy.storchakasetmessages: + msg160122
title: Add {encode,decode}_filter_properties() functions to lzma module -> Add {encode, decode}_filter_properties() functions to lzma module
2012-05-07 05:11:22nadeem.vawdasetmessages: + msg160118
2012-05-06 23:05:27serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg160115
2012-05-06 21:10:03nadeem.vawdasetstatus: open -> closed
resolution: fixed
messages: + msg160108

stage: patch review -> resolved
2012-05-06 10:38:56pitrousetmessages: + msg160073
2012-05-06 10:34:48nadeem.vawdasetfiles: + lzma-properties.diff

messages: + msg160071
2012-05-06 08:27:43pitrousetnosy: + pitrou
messages: + msg160063
2012-05-06 01:08:03nadeem.vawdacreate