Issue 1625: bz2.BZ2File doesn't support multiple streams

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s :	save comment
<Esc> :	cancel edit

#1625: bz2.BZ2File doesn't support multiple streams

Can't Edit
Can't Publish+Mail
Start Review

Created:
2 years ago by therve

Modified:
2 years ago

Reviewers:
merwok

CC:
loewis, akuchling, gustavo_niemeyer.net, jcea, AntoinePitrou, therve_free.fr, haypo, python_tomlee.co, nadeem.vawda, dirkjan_ochtman.nl, eric.araujo, nirai, r.david.murray, dbonner_vmware.com, ozancag_gmail.com, Oliver.Deppert_stud.tu-darmstadt.de, devnull_psf.upfronthosting.co.za, gokcen.eraslan_gmail.com, retrogradeorbit_gmail.com, devnull_psf.upfronthosting.co.za, gokcen_pardus.org.tr

Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 3

Created: 2 years ago

Download raw patch set

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats			Patch
		Lib/bz2.py	View		3 chunks	+36 lines, -11 lines	1 comment	Download
		Lib/test/test_bz2.py	View	1	14 chunks	+124 lines, -2 lines	2 comments	Download

Messages

Total messages: 1

Expand All Messages | Collapse All Messages

Looks good.  I only have one paranoid comment: since the tests use self.TEXT * 5
to create multiple streams, the ordering of the files is not tested.  IOW, the
code could mix-up the order of the files and the tests would not catch that.  Is
it a concern?

http://bugs.python.org/review/1625/diff/2695/6526
File Lib/bz2.py (right):

http://bugs.python.org/review/1625/diff/2695/6526#newcode409
Lib/bz2.py:409: result += decomp.decompress(data)
Is this efficient?  I understood that other Python implementations had poorly
performing str.__iadd__, and therefore that using a list was the common idiom
(using “return b''.join(result)” at the end).

http://bugs.python.org/review/1625/diff/2695/6527
File Lib/test/test_bz2.py (right):

http://bugs.python.org/review/1625/diff/2695/6527#newcode99
Lib/test/test_bz2.py:99: # "Test BZ2File.read() with a multi stream archive"
The comment seems unnecessary, as the method name is clear enough.  (I see that
the existing code has those weird-looking, redundant comments, but you don’t
have to do the same.)

http://bugs.python.org/review/1625/diff/2695/6527#newcode142
Lib/test/test_bz2.py:142: while 1:
while True:

Expand All Messages | Collapse All Messages