[Python-Dev] Need some patches checked

Guido van Rossum guido@python.org
Sun, 11 May 2003 20:20:18 -0400


> Since I am trying to tackle patches that were not written by me for the 
> first time I need someone to check that I am doing the right thing.
> 
> http://www.python.org/sf/649742 is a patch to make adding headers to 
> urllib2's Request object have consistent case.  I cleaned up the patch 
> and everything seems reasonable and I don't see how doing this will hurt 
> backwards-compatibilty short of code that tried to add multiple headers 
> of the same name with different case which is not legal anyway for HTTP.

Good!  I just noticed with disgust that the headers dict is case
currently case-sensitive, so that if I want to change the
Content-type header, I have to use the exact case used in the source.
I can't imagine b/w compatibility issues with this.

> http://www.python.org/sf/639139 is a patch wanting to remove an 
> isinstance assertion.  Raymond initially suggested weakening the 
> assertion to doing attribute checks.  I personally see no reason we 
> can't just take the check out entirely since the code does not appear to 
> have any place where it will mask an AttributeError exception and the 
> comment for the assert says it is just for checking the interface.  But 
> since Raymond initially wanted to go another direction I need someone to 
> step in and give me some advice (or Raymond can look at it again; patch 
> is old).

The advantage of the assert (or some other check) is to catch a type
error early, rather than 4 call levels deeper, where the source of the
AttributeError may not be obvious when it happens.  But I agree that
that is a minor issue, and for correct code removing the assert is
fine.  Checking exactly for the attributes that are (or may be) used
is probably overly expensive.

--Guido van Rossum (home page: http://www.python.org/~guido/)