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

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

Can't Edit
Can't Publish+Mail
Start Review
10 years, 10 months ago by Nick Coghlan
10 years, 10 months ago
Antoine Pitrou
Base URL:

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


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)

File Lib/urllib/parse.py (right):

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.

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