Before commenting on the patch itself, I'd like to comment on the patch describing it. I'm missing a discussion as to why weak references don't act as proxies (or why they do now). A weak proxy would provide the same attributes as the object which it encapsulates, so it could be used transparently in place of the original object. I can think of a number of reasons why it is not done this way (e.g. complete transparency is impossible to achieve); now that a revision of the patch provides proxies, the documentation should state which features are forwarded to the proxy and which aren't (it lists the type() as a difference, but I doubt that is the only difference - repr is also different). Next, I wonder whether weakref.new is allowed to return an existing weak reference to the same object. If that is not acceptable, I'd like to know why - if it was acceptable, then weakref.new(instance) (i.e. without callback) could return the same weak reference all the time. A smart implementation might chose to put the weak reference with no callback in the start of the list, so creation of additional weak references to the same object would be inexpensive. Likewise, I'd like to know the rationale for the clear method. Why is it desirable to drop the object, yet keep the weak reference? Isn't it easier for the application to either ignore clearing altogether, or dropping the reference to the weak reference? So I'd propose to kill the clear method. Again on proxies, there is no discussion or documentation of the ReferenceError. Why is it a RuntimeError? LookupError, ValueError, and AttributeError seem to be just as fine or better. On to the type type extensions: Should there be a type flag indicating presence of tp_weaklistoffset? It appears that the type structure had tp_xxx7 for a long time, so likely all in-use binary modules have that field set to zero. Is that sufficient? Thanks for reading all of this message, Martin
I'll leave most of this to Fred, but I'll reply to two items (Fred can add these replies to the PEP):
RuntimeError was my suggestion. The error doesn't really qualify as a LookupError in my view (there's no key that could be valid or invalid) and ValueError seems too general (that's typically used for out-of-range arguments and unparseable strings and the like). Do you have a reason why RuntimeError is inappropriate?
Yes, that should be sufficient. (I'm also going to clain tp_xxx7 for the rich comparison function slot, but either patch can be modified to use tp_xxx8 instead.) Maybe it's time to add a bunch of new spares?
Thanks for reading all of this message,
You're welcome. --Guido van Rossum (home page: http://www.python.org/~guido/)
I'll leave most of this to Fred, but I'll reply to two items (Fred can add these replies to the PEP):
RuntimeError was my suggestion. The error doesn't really qualify as a LookupError in my view (there's no key that could be valid or invalid) and ValueError seems too general (that's typically used for out-of-range arguments and unparseable strings and the like). Do you have a reason why RuntimeError is inappropriate?
Yes, that should be sufficient. (I'm also going to clain tp_xxx7 for the rich comparison function slot, but either patch can be modified to use tp_xxx8 instead.) Maybe it's time to add a bunch of new spares?
Thanks for reading all of this message,
You're welcome. --Guido van Rossum (home page: http://www.python.org/~guido/)
participants (2)
-
Guido van Rossum -
Martin v. Loewis