[Patches] [ python-Patches-639139 ] Remove type-check from urllib2
SourceForge.net
noreply@sourceforge.net
Mon, 12 May 2003 00:32:21 -0700
Patches item #639139, was opened at 2002-11-15 14:25
Message generated for change (Comment added) made by bcannon
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=639139&group_id=5470
Category: Library (Lib)
Group: Python 2.3
>Status: Closed
Resolution: Accepted
Priority: 3
Submitted By: John J Lee (jjlee)
Assigned to: Brett Cannon (bcannon)
Summary: Remove type-check from urllib2
Initial Comment:
Remove undesirable type-checking assertion from
urllib2.Request.
----------------------------------------------------------------------
>Comment By: Brett Cannon (bcannon)
Date: 2003-05-12 00:32
Message:
Logged In: YES
user_id=357491
Applied as Lib/urllib2.py 1.44
----------------------------------------------------------------------
Comment By: Brett Cannon (bcannon)
Date: 2003-05-11 23:55
Message:
Logged In: YES
user_id=357491
Too late, I got approval. =) I have to anyway, John, since this is one of the
first patches I have handled personally. The assert will be removed some
time this week.
----------------------------------------------------------------------
Comment By: John J Lee (jjlee)
Date: 2003-05-11 17:59
Message:
Logged In: YES
user_id=261020
Oh, is it really worth any more of everyone's time,
I wonder? I didn't anticipate a big discussion.
Let's just leave it at the status-quo, shell we?
----------------------------------------------------------------------
Comment By: Brett Cannon (bcannon)
Date: 2003-05-11 16:24
Message:
Logged In: YES
user_id=357491
I am going to see if i can get someone to step in here and give me some
advice on how to handle this. I personally have no issues ditching the check
entirely. I just checked the module and it does not look like an
AttributeError will be caught somewhere else in the code.
But since Raymond only wanted to weaken the assertion and not remove it I
feel the need to get someone else to choose a side on this to force which
resolution is taken (status quo, hasattr, remove the check entirely).
----------------------------------------------------------------------
Comment By: John J Lee (jjlee)
Date: 2003-05-11 04:46
Message:
Logged In: YES
user_id=261020
No!! Ahem. Personally I wouldn't want to apply a
patch like that -- I think it's inappropriate, since this is
a violated precondition, and the patch you describe
would add significant complexity (not to mention
inefficiency, if I dare use that word here).
The caller will get AttributeError anyway, with my
patch, and though it wasn't my goal to pamper
callers who mess up the call, that's actually not an
unreasonable error.
Would it convince you if I pointed out that my
patch is simply EAFP again, at a different place in
the call stack? If you don't like AttributeError
being raised, you could always wrap up the five
uses of req in try: except: blocks, but personally
I don't think that's worth the added complexity
or bug-masking risk.
I don't know... first he tells me not to attribute
check, then he wants to do it in a loop ;-)
----------------------------------------------------------------------
Comment By: Brett Cannon (bcannon)
Date: 2003-05-10 22:33
Message:
Logged In: YES
user_id=357491
This is not what I meant by EAFP. In order for a patch to do that you would
need to wrap all outgoing calls in that method that will use 'req' and see if
the exception was because the object didn't have the proper interface. Good
try, though. =)
Doing it using hasattr in a loop is probably going to be the best bet. If you
don't want to do the patch I will understand; I should have been more clear
by what I was expecting for EAFP. Thus if you don't want to do it I will do
the patch myself.
----------------------------------------------------------------------
Comment By: John J Lee (jjlee)
Date: 2003-05-10 04:58
Message:
Logged In: YES
user_id=261020
Is there a 'least interesting patch of the year' prize?
My entry is attached.
----------------------------------------------------------------------
Comment By: Brett Cannon (bcannon)
Date: 2003-05-09 16:47
Message:
Logged In: YES
user_id=357491
=) OK. If you care to rewrite the patch using EAFP, then please do so. If
not, then you just followup here saying you don't think the patch is worthing
persuing any farther so I can close it?
----------------------------------------------------------------------
Comment By: John J Lee (jjlee)
Date: 2003-05-09 05:43
Message:
Logged In: YES
user_id=261020
Well, I think EAFP is probably best.
An attribute check is in some sense the nearest thing to the assert
that's there now, which is why my patch did that. OTOH, a method
check seemed half-baked to me and so worse than an attribute check.
<shrug> it's not worth *too* much of our time :-)
----------------------------------------------------------------------
Comment By: Brett Cannon (bcannon)
Date: 2003-05-08 18:32
Message:
Logged In: YES
user_id=357491
It's not so much that the dummy attribute would break anything so much as
it is just unneeded. If you want to make sure an object is not going to meet
your required API you either just follow EAFP (Easier to Ask Forgiveness than
Permission) or you explicitly test for the required interface. There is no good
argument to have to flag an object to say that it meets an API spec.
----------------------------------------------------------------------
Comment By: John J Lee (jjlee)
Date: 2002-11-17 13:21
Message:
Logged In: YES
user_id=261020
Why not a new attribute? What would it break?
Checking for the interface by checking all the methods
(there are maybe ten of them) is not really practical,
and really it's the intent that's the important bit.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-11-17 09:57
Message:
Logged In: YES
user_id=80475
I see no problem with weakening the assertion, but
hasattr should check for a required part of the interface
instead of a new, undocumented, dummy attribute.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2002-11-17 09:55
Message:
Logged In: YES
user_id=80475
I see no problem with weakening the assertion, but
hasattr should check for a required part of the interface
instead of a new, undocumented, dummy attribute.
----------------------------------------------------------------------
Comment By: John J Lee (jjlee)
Date: 2002-11-17 06:46
Message:
Logged In: YES
user_id=261020
It's widely regarded as a bug if Python code checks for type
with
isinstance (or type(foo) == type(bar)) without some good
reason.
It's plausible that you may want to make an object that
implements the Request interface without deriving from
Request
(say, I don't know, to implement the frobozz URI scheme,
which
requires ordered headers, and never has any data associated
with
it). If so, you don't want to have to follow 'bug fixes' in
the
Python std. library that may break your code simply because
you
had to derive from Request to satisfy the assertion. I
might
have done this when I wrote a couple of modules that build
on
urllib2, actually. I'm not sure whether that would have
been the
best way, because I didn't think about it since I didn't
have any
choice in the matter, thanks to this assertion!
OTOH, it's true that removing type-checks can break
backwards
compatibility. However, this is an assertion, not a real
runtime
type-check, so it won't break backwards compatibility: if
people
are relying on catching AssertionError to do type-checking
in
their own code, that's their problem!
The docs say:
urlopen(url[, data])
Open the URL url, which can be either a string or a Request
object (currently the code checks that it really is a
Request
instance, or an instance of a subclass of Request).
Note the 'currently' (and the source code comment indicating
that
what we really want to check is the interface), and that
fact
that the code *doesn't* actually check it, but only asserts.
Request interface is already documented, so there's no
problem
there.
John
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-11-15 15:14
Message:
Logged In: YES
user_id=33168
John, could you explain why you need it and what is the benefit?
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=639139&group_id=5470