Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(114)

Issue 2318041: Python Issue 9873 - bytes in urllib.parse

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 10 months ago by Nick Coghlan
Modified:
10 years, 10 months ago
Reviewers:
Antoine Pitrou
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+173 lines, -91 lines) Patch
M Lib/test/test_urlparse.py View 6 chunks +36 lines, -10 lines 0 comments Download
M Lib/urllib/parse.py View 10 chunks +137 lines, -81 lines 2 comments Download

Messages

Total messages: 1
Antoine Pitrou
10 years, 10 months ago (2010-09-29 21:18:08 UTC) #1
Overall the patch looks ok, although I can't say the implementation strategy
makes for very readable code.

(I would suggest to temporarily decode bytes using latin1, but the properties on
ResultMixin wouldn't make that very pretty either)

http://codereview.appspot.com/2318041/diff/1/Lib/urllib/parse.py
File Lib/urllib/parse.py (right):

http://codereview.appspot.com/2318041/diff/1/Lib/urllib/parse.py#newcode48
Lib/urllib/parse.py:48: uses_params = ['ftp', 'hdl', 'prospero', 'http', 'imap',
Not directly related to the patch, but shoudn't all these lists be sets? It
would speed up containment tests quite a bit.

http://codereview.appspot.com/2318041/diff/1/Lib/urllib/parse.py#newcode139
Lib/urllib/parse.py:139: __slots__ = ()
Again, not related to the patch, but ResultMixin lacks a __slots__, so
SplitResult will have a __dict__ anyway.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b