[Python-checkins] r80101 - in python/trunk: Lib/test/test_urlparse.py Lib/urlparse.py Misc/NEWS

Senthil Kumaran orsenthil at gmail.com
Tue Apr 20 23:03:14 CEST 2010


On Mon, Apr 19, 2010 at 11:31:00AM +0300, Ezio Melotti wrote:
> Hi,
> 

Thank you for the review comments.
> 
> For all these 3 invalid URLs the exception is raised by
> urlparse.urlparse(), so the one raised by .hostname is not tested
> (and the two assertRaises are equivalent).

With the recent change, yes this is true. I can remove the .hostname
check itself.

> >
> >  def _splitnetloc(url, start=0):
> >      delim = len(url)   # position of end of domain part of url, default is end
> >+    if '[' in url:     # check for invalid IPv6 URL
> >+        if not ']' in url: raise ValueError("Invalid IPv6 URL")
> >+    elif ']' in url:
> >+        if not '[' in url: raise ValueError("Invalid IPv6 URL")
> 
> Isn't this equivalent to:
> if (('[' in url and ']' not in url) or
>     (']' in url and '[' not in url)):
>     raise ValueError("Invalid IPv6 URL")
> I find this version more readable and it avoids 4 if and the
> duplication of the raise. The 'delim = len(url)' can also be moved
> after this check.

Yes again. The second version is more readable. I shall use this one.

> 
> This should also be documented and have a versionchanged added.

Yes. Shall note to add the versionchanged and docs.

-- 
Senthil



More information about the Python-checkins mailing list