Reviewers: nadeem vawda <nadeem.vawda_gmail.com>,

http://codereview.appspot.com/4274045/diff/1/Lib/bz2.py
File Lib/bz2.py (right):

http://codereview.appspot.com/4274045/diff/1/Lib/bz2.py#newcode25
Lib/bz2.py:25: class BZ2File:
Is there any reason it doesn't inherit io.BufferedIOBase?
(it should also bring you a couple of methods implemented for free:
readlines, writelines, __iter__, __next__, __enter__, __exit__)

You should probably also implement fileno() (simply return
self.fp.fileno()) and the `closed` property.

http://codereview.appspot.com/4274045/diff/1/Lib/bz2.py#newcode386
Lib/bz2.py:386: class BZ2Compressor:
I don't think there's a point in a Python wrapper, since the wrapper is
so trivial. Just do the lock operations in C.
Same for BZ2Decompressor.

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c
File Modules/_bz2module.c (left):

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c#oldcode123
Modules/_bz2module.c:123: #ifdef WITH_THREAD
As mentioned in Lib/bz2.py, I would keep the lock on the C side since it
isn't significantly more complicated, and it avoids having to write a
Python wrapper around the compressor and decompressor types.

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c
File Modules/_bz2module.c (right):

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c#newcode3
Modules/_bz2module.c:3: #include "Python.h"
Since this is a new start, perhaps we should add
   #define PY_SSIZE_T_CLEAN
before including "Python.h"?
This will ensure no code in the module will rely on the old behaviour.

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c#newcode48
Modules/_bz2module.c:48: "libbzip2 was not compiled correctly");
Just a nit, but I'm not sure there's any point in renaming "the bz2
library" to "libbzip2"?
(also, under Windows I'm not sure the library naming convention is the
same as under Unix)

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c#newcode78
Modules/_bz2module.c:78: "Unrecognized error from libbzip2: %d",
bzerror);
Out of curiousity, did you encounter this condition?

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c#newcode122
Modules/_bz2module.c:122: c->bzs.avail_out = PyBytes_GET_SIZE(result);
Do note that avail_in and avail_out are 32-bit ints, and therefore this
is not 64-bit clean. I guess you're just copying the old code here, but
that would deserve a separate patch later. Perhaps add a comment in the
meantime.

http://codereview.appspot.com/4274045/diff/1/Modules/_bz2module.c#newcode209
Modules/_bz2module.c:209: "Provide a block of data to the compressor."},
You could instead re-use the old, more precise docstrings. Also, using
PyDoc_STRVAR is preferred so as to make it easier to modify multi-line
docstrings.

http://codereview.appspot.com/4274045/diff/1/setup.py
File setup.py (right):

http://codereview.appspot.com/4274045/diff/1/setup.py#newcode1236
setup.py:1236: exts.append( Extension('_bz2', ['_bz2module.c'],
The Windows build files probably need updating as well. Can you do it?
Otherwise I'll have a try.

Please review this at http://codereview.appspot.com/4274045/

Affected files:
   A Lib/bz2.py
   M Lib/test/test_bz2.py
   M Modules/_bz2module.c
   M setup.py