| msg57828 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2007-11-25 13:31 |
| Perhaps it would be good to move the rest of classobject.c into methodobject.c after that. |
|
|
| msg57841 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-26 17:05 |
| The new patch adds new.boundcfunction as a replacement for new.instancemethod(id, None, cls). I'm going to add docs if you like the approach. |
|
|
| msg57846 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-11-26 18:49 |
| Note though that the "new" module was deprecated once... |
|
|
| msg57847 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-26 19:04 |
| OK. Some code review comments: - Please clean up the comment in classobject.c starting with "Method objects are used for one purposes:" -- to begin with, "one purposes" is ungrammatical. Best to remove the (a) bullet and rephrase the whole thing more like "Method objects are used for bound instance methods (...)" - The error "unbound methods are not supported" sounds a bit strange; better rephrase more positive as "self must not be None" - There is still a comment left "No, it is an unbound method". Is this code still reachable? I though all ways to set im_self to NULL/None are blocked? - Is bug 1202533 still worth testing for in test_descr.py? I don't know that using a lambda reproduces the error condition that once existed. Functional suggestions: - Do we really need im_class for anything any more? ISTM that the one place where it is still used (in method_repr), we could as well use the class of im_self. (And before you think of super() as a counter-argument: super() passes the object's class in rather than the class where the method is found (though this may be considered a bug). - I think that, like func_code -> __code__, the im_xxx attributes should be renamed __xxx__. The 'new' module claims to exist solely for backwards compatibility. If that's true, why are we adding to it? In any case, the _BoundCFunction() class is redundant -- you can just use the "method" type, which is easily accessed as any bound method's __class__ attribute. And is there a use case for an *unbound* C function? If not, you could replace boundcfunction() with just a call to the method type. |
|
|
| msg57868 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-27 10:37 |
| Guido van Rossum wrote: > - Please clean up the comment in classobject.c starting with "Method > objects are used for one purposes:" -- to begin with, "one purposes" is > ungrammatical. Best to remove the (a) bullet and rephrase the whole > thing more like "Method objects are used for bound instance methods (...)" Done > - The error "unbound methods are not supported" sounds a bit strange; > better rephrase more positive as "self must not be None" Done. Didn't I say that I'm not good with short, precise error messages? :) > - There is still a comment left "No, it is an unbound method". Is this > code still reachable? I though all ways to set im_self to NULL/None are > blocked? It's not longer reachable but I left it there to catch problems. It's gone now. > > - Is bug 1202533 still worth testing for in test_descr.py? I don't know > that using a lambda reproduces the error condition that once existed. Removed > - Do we really need im_class for anything any more? ISTM that the one > place where it is still used (in method_repr), we could as well use the > class of im_self. (And before you think of super() as a > counter-argument: super() passes the object's class in rather than the > class where the method is found (though this may be considered a bug). You are right. im_class can be substituted with __self__.__class__. I've also removed the class argument from PyMethod_New(). It's not required anymore. > - I think that, like func_code -> __code__, the im_xxx attributes should > be renamed __xxx__. Done im_func -> __func__ im_self -> __self__ im_class -> removed I've left the names in the struct untouched. > The 'new' module claims to exist solely for backwards compatibility. If > that's true, why are we adding to it? In any case, the > _BoundCFunction() class is redundant -- you can just use the "method" > type, which is easily accessed as any bound method's __class__ > attribute. And is there a use case for an *unbound* C function? If not, > you could replace boundcfunction() with just a call to the method type. How could a bind a builtin method to a class so that instance().builtin() gets self as first argument? In one unit test the builtin function id() is bound to a class: class Example: id = somemagic(id) Example().id() results in id(Example()) I can't get the same effect with MethodType because it binds only to instances, not to classes. Christian |
|
|
| msg57869 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-11-27 10:42 |
| I've committed the changes in r59189. The documentation still needs updates. I had only time to fix some of the docs. |
|
|
| msg58000 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-11-30 19:29 |
| Reducing priority and changing assignee since this is now just waiting for a doc update. |
|
|
| msg58871 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-12-20 13:12 |
| Georg, do we need more docs? |
|
|
| msg58936 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-12-21 08:17 |
| As far as I can see, not really. |
|
|