instancemethod_getattro seems to be partially wrong
Hi again, again! After hours of investigating why my instance method __reduce__ doesn't work, I found out the following: instancemethod_getattro does this: if (PyType_HasFeature(tp, Py_TPFLAGS_HAVE_CLASS)) { if (tp->tp_dict == NULL) { if (PyType_Ready(tp) < 0) return NULL; } descr = _PyType_Lookup(tp, name); } f = NULL; if (descr != NULL) { f = TP_DESCR_GET(descr->ob_type); if (f != NULL && PyDescr_IsData(descr)) return f(descr, obj, (PyObject *)obj->ob_type); } Why, please can someone explain, why does it ask for PyDescr_IsData ??? I think this is wrong. I'm defining an __reduce__ method, and it doesn't provide a tp_descr_set, as defined in... int PyDescr_IsData(PyObject *d) { return d->ob_type->tp_descr_set != NULL; } but for what reason is this required??? This thingie is going wrong both in Py 2.2.3 and in Py 2.3.2, so I guess there is something very basically going wrong. I'd like to fix that, but I need to understand what the intent of this code has been. Can somebody, perhaps the author, explain why this is this way? thanks so much -- chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
instancemethod_getattro
does this:
if (PyType_HasFeature(tp, Py_TPFLAGS_HAVE_CLASS)) { if (tp->tp_dict == NULL) { if (PyType_Ready(tp) < 0) return NULL; } descr = _PyType_Lookup(tp, name); }
f = NULL; if (descr != NULL) { f = TP_DESCR_GET(descr->ob_type); if (f != NULL && PyDescr_IsData(descr)) return f(descr, obj, (PyObject *)obj->ob_type); }
[...] why does it ask for PyDescr_IsData ???
It's the general pattern: a data descriptor on the class can override
an attribute on the instance, but a method descriptor cannot. You'll
find this in PyObject_Generic{Get,Set}Attr() too, and in type_getattro().
This is so that if you define a method in a class, you can override it
by setting an instance variable of the same name; this was always
possible for classic classes and I don't see why it shouldn't work for
new-style classes. But it should also be possible to put a descriptor
on the class that takes complete control.
The case you quote is about delegating bound method attributes to
function attributes, but the same reasoning applies generally, I would
think: unless the descriptor is a data descriptor, the function
attribute should have precedence, IOW a function attribute should be
able to override a method on a bound instance.
Here's an example of the difference:
class C:
def f(s): pass
f.__repr__ = lambda: "42"
print C().f.__repr__()
This prints "42". If you comment out the PyDescr_IsData() call, it
will print "
Guido van Rossum wrote: ...
Here's an example of the difference:
class C: def f(s): pass f.__repr__ = lambda: "42" print C().f.__repr__()
This prints "42". If you comment out the PyDescr_IsData() call, it will print "
>". I'm not entirely clear what goes wrong in your case.
Well, in my case, I try to pickle a bound method, so I expect that C().f.__reduce__ gives me a reasonable object: A method of an instance of C that is able to do an __reduce__, that is, I need the bound f and try to get its __reduce__ in a bound way. If that's not the way to do it, which is it? thanks - chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
Here's an example of the difference:
class C: def f(s): pass f.__repr__ = lambda: "42" print C().f.__repr__()
This prints "42". If you comment out the PyDescr_IsData() call, it will print "
>". I'm not entirely clear what goes wrong in your case.
Well, in my case, I try to pickle a bound method, so
Um, my brain just did a double-take. Standard Python doesn't let you do that, so you must be changing some internals. Which parts of Python are you trying to change and which parts are you trying to keep unchanged? If you were using a different metaclass you could just create a different implementation of instancemethod that does what you want, so apparently you're not going that route. (With new-style classes, instancemethod isn't that special any more -- it's just a currying construct with some extra baggage.)
I expect that C().f.__reduce__ gives me a reasonable object: A method of an instance of C that is able to do an __reduce__, that is, I need the bound f and try to get its __reduce__ in a bound way.
Try again. I don't think that C().f.__reduce__ should be a method of an instance of C. You want it to be a method of a bound method object, right?
If that's not the way to do it, which is it?
I think what I suggested above -- forget about the existing instancemethod implementation. But I really don't understand the context in which you are doing this well enough to give you advice, and in any context that I understand the whole construct doesn't make sense. :-( --Guido van Rossum (home page: http://www.python.org/~guido/)
Hi Guido, ...
Um, my brain just did a double-take. Standard Python doesn't let you do that, so you must be changing some internals. Which parts of Python are you trying to change and which parts are you trying to keep unchanged? If you were using a different metaclass you could just create a different implementation of instancemethod that does what you want, so apparently you're not going that route. (With new-style classes, instancemethod isn't that special any more -- it's just a currying construct with some extra baggage.)
No no no, I'm not fiddling around with any internals, here. I just want to use the machinary as it is, and to be able to pickle almost everything. So, if somebody did a v=C().x, I have that variable around. In order to pickle it, I ask for its __reduce__, or in other words, I don't ask for it, I try to supply it, so the pickling engine can find it. My expectation is that C().x.__reduce__ gives me the bound __reduce__ method of the bound x method of a C instance. ...
Try again. I don't think that C().f.__reduce__ should be a method of an instance of C. You want it to be a method of a bound method object, right?
No, __reduce__ is a method of f, which is bound to an instance of C. Calling it will give me what I need to pickle the bound f method. This is all what I want. I think this is just natural.
If that's not the way to do it, which is it?
I think what I suggested above -- forget about the existing instancemethod implementation. But I really don't understand the context in which you are doing this well enough to give you advice, and in any context that I understand the whole construct doesn't make sense. :-(
Once again. What I try to achieve is complete thread pickling. That means, I need to supply pickling methods to all objects which don't have builtin support in cPickle or which don't provide __reduce__ already. I have done this for some 10 or more types, successfully. Bound PyCFunction objects are nice and don't give me a problem. Bound PyFunction objects do give me a problem, since they don't want to give me what they are bound to. My options are: - Do an ugly patch that special cases for __reduce__, which I did just now, in order to seet hings working. - get the master's voice about how to do this generally right, and do it generally right. I would of course prefer the latter, but I also try to save as much time as I can while supporting my clients, since Stackless is almost no longer sponsored, and I have money problems. thanks so much -- chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
Hi Guido,
...
Um, my brain just did a double-take. Standard Python doesn't let you do that, so you must be changing some internals. Which parts of Python are you trying to change and which parts are you trying to keep unchanged? If you were using a different metaclass you could just create a different implementation of instancemethod that does what you want, so apparently you're not going that route. (With new-style classes, instancemethod isn't that special any more -- it's just a currying construct with some extra baggage.)
No no no, I'm not fiddling around with any internals, here. I just want to use the machinary as it is, and to be able to pickle almost everything.
So, if somebody did a v=C().x, I have that variable around. In order to pickle it, I ask for its __reduce__, or in other words, I don't ask for it, I try to supply it, so the pickling engine can find it.
But how, I wonder, are you providing it? You can't subclass instancemethod -- how do you manage to add a __reduce__ method to it without fiddling with any internals?
My expectation is that C().x.__reduce__ gives me the bound __reduce__ method of the bound x method of a C instance.
Yes, unfortunately you get the __reduce__ method of the unbound function instead. I think Martin is right: copy_reg may be your last hope. (Or subclassing pickle to special-case instancemethod.) The pickling machinery wasn't intended to pickle bound methods or functions etc., and doesn't particularly go out of its way to allow you to add that functionality.
...
Try again. I don't think that C().f.__reduce__ should be a method of an instance of C. You want it to be a method of a bound method object, right?
No, __reduce__ is a method of f, which is bound to an instance of C. Calling it will give me what I need to pickle the bound f method. This is all what I want. I think this is just natural.
And it would be except for the delegation of method attributes to function attributes. It is a similar aliasing problem as you see when you try to access the __getattr__ implementation for classes as C.__getattr__ -- you get the __getattr__ for C instances instead. So you have to use type(C).__getattr__ instead. That would work for __reduce__ too I think: new.instancemethod.__reduce__(C().f).
If that's not the way to do it, which is it?
I think what I suggested above -- forget about the existing instancemethod implementation. But I really don't understand the context in which you are doing this well enough to give you advice, and in any context that I understand the whole construct doesn't make sense. :-(
Once again. What I try to achieve is complete thread pickling. That means, I need to supply pickling methods to all objects which don't have builtin support in cPickle or which don't provide __reduce__ already. I have done this for some 10 or more types, successfully. Bound PyCFunction objects are nice and don't give me a problem. Bound PyFunction objects do give me a problem, since they don't want to give me what they are bound to.
OK, so you *are* messing with internals after all (== changing C code), right? Or else how do you accomplish this?
My options are: - Do an ugly patch that special cases for __reduce__, which I did just now, in order to seet hings working. - get the master's voice about how to do this generally right, and do it generally right.
I would of course prefer the latter, but I also try to save as much time as I can while supporting my clients, since Stackless is almost no longer sponsored, and I have money problems.
I have a real job too, that's why I have little time to help you. :-( --Guido van Rossum (home page: http://www.python.org/~guido/)
Hi again, Guido,
No no no, I'm not fiddling around with any internals, here. I just want to use the machinary as it is, and to be able to pickle almost everything.
Sorry, this was a lie. Sure I'm fiddling internaly, but simply by installing some __reduce__ methids, hoping that they work. This worked most of the time, but I'm having problems with bound methods.
But how, I wonder, are you providing it? You can't subclass instancemethod -- how do you manage to add a __reduce__ method to it without fiddling with any internals?
I added __reduce__ to the PyMethod type and tried to figure out why it didn't take it.
My expectation is that C().x.__reduce__ gives me the bound __reduce__ method of the bound x method of a C instance.
Yes, unfortunately you get the __reduce__ method of the unbound function instead.
I think Martin is right: copy_reg may be your last hope. (Or subclassing pickle to special-case instancemethod.)
Well, I see your point, but please let me explain mine, again: If there is a class C which has a method x, then C().x is a perfectly fine expression, yielding a bound method. If I now like to pickle this expression, I would use the __reduce__ protocol and ask C().x for its __reduce__ property. Now, please see that __reduce__ has no parameters, i.e. it has no other chance to do the right thing(TM) but by relying on to be bound to the right thing. So, doesn't it make sense to have __reduce__ to be always returned as a method of some bound anything? In other words, shouldn't things that are only useful as bound things, always be bound?
The pickling machinery wasn't intended to pickle bound methods or functions etc., and doesn't particularly go out of its way to allow you to add that functionality.
The pickling machinery gives me an __reduce__ interface, and I'm expecting that this is able to pickle everything. ...
And it would be except for the delegation of method attributes to function attributes. It is a similar aliasing problem as you see when you try to access the __getattr__ implementation for classes as C.__getattr__ -- you get the __getattr__ for C instances instead. So you have to use type(C).__getattr__ instead. That would work for __reduce__ too I think: new.instancemethod.__reduce__(C().f).
I agree! But I can't do this in this context, using __reduce__ only. In other words, I'd have to add stuff to copyreg.py, which I tried to circumvent. ...
OK, so you *are* messing with internals after all (== changing C code), right? Or else how do you accomplish this?
Yessir, I'm augmenting all things-to-be-pickled with __reduce__ methods. And this time is the first time that it doesn't work. ...
I have a real job too, that's why I have little time to help you. :-(
I agree (and I didn't ask *you* in the first place), but still I'd like to ask the general question: Is this really the right way to handle bound objects? Is the is_data criterion correct? If I am asking for an attribute that makes *only* sense if it is bound, like in the parameter-less __reduce__ case, wouldn't it be the correct behavior to give me that bound object? I have the strong impression that there is some difference in methods which isn't dealt with, correctly, at the moment. If a method wants to be bound to something, it should be get bound to something. Especially, if this method is useless without being bound. Please, swallow this idea a little bit, before rejecting it. I think that "is_data" is too rough and doesn't fit the requirements, all the time. sincerely -- chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
Summary: Chistian is right after all. instancemethod_getattro should always prefer bound method attributes over function attributes.
No no no, I'm not fiddling around with any internals, here. I just want to use the machinary as it is, and to be able to pickle almost everything.
Sorry, this was a lie.
Sigh. OK, you're forgiven.
Sure I'm fiddling internaly, but simply by installing some __reduce__ methids, hoping that they work.
OK, so you *could* just make the change you want, but you are asking why it isn't like that in the first place. Good idea...
This worked most of the time, but I'm having problems with bound methods.
We've established that without a doubt, yes. :-)
But how, I wonder, are you providing it? You can't subclass instancemethod -- how do you manage to add a __reduce__ method to it without fiddling with any internals?
I added __reduce__ to the PyMethod type and tried to figure out why it didn't take it.
OK. Stating that upfront would have helped...
My expectation is that C().x.__reduce__ gives me the bound __reduce__ method of the bound x method of a C instance.
Yes, unfortunately you get the __reduce__ method of the unbound function instead.
I think Martin is right: copy_reg may be your last hope. (Or subclassing pickle to special-case instancemethod.)
Well, I see your point, but please let me explain mine, again: If there is a class C which has a method x, then C().x is a perfectly fine expression, yielding a bound method.
Of course.
If I now like to pickle this expression, I would use the __reduce__ protocol and ask C().x for its __reduce__ property.
Which unfortunately gets the __reduce__ property of the underlying *function* object (also named x) used to implement the method. This function can be accessed as C.__dict__['x']. (Not as C.x, that returns an unbound method object, which is the same kind of object as a bound method object but without an instance. :-)
Now, please see that __reduce__ has no parameters, i.e. it has no other chance to do the right thing(TM) but by relying on to be bound to the right thing. So, doesn't it make sense to have __reduce__ to be always returned as a method of some bound anything?
In other words, shouldn't things that are only useful as bound things, always be bound?
This question doesn't address the real issue, which is the attribute
delegation to the underlying function object.
What *should* happen when the same attribute name exists on the
function and on the bound method? In 2.1, when function attributes
were first introduced, this was easy: a few attributes were special
for the bound method (im_func, im_self, im_class) and for these the
bound method attribute wins (if you set an attribute with one of those
names on the function, you can't access it through the bound method).
The *intention* was for the 2.2 version to have the same behavior:
only im_func, im_self and im_class would be handled by the bound
method, other attributes would be handled by the function object.
This is what the IsData test is attempting to do -- the im_*
attributes are represented by data descriptors now. The __class__
attribute is also a data descriptor, so that C().x.__class__ gives us
The pickling machinery wasn't intended to pickle bound methods or functions etc., and doesn't particularly go out of its way to allow you to add that functionality.
The pickling machinery gives me an __reduce__ interface, and I'm expecting that this is able to pickle everything.
I don't think you'd have a chance of pickle classes if you only relied on __reduce__. Fortunately there are other mechanisms. :-) (I wonder if the pickling code shouldn't try to call x.__class__.__reduce__(x) rather than x.__reduce__() -- then none of these problems would have occurred... :-)
...
And it would be except for the delegation of method attributes to function attributes. It is a similar aliasing problem as you see when you try to access the __getattr__ implementation for classes as C.__getattr__ -- you get the __getattr__ for C instances instead. So you have to use type(C).__getattr__ instead. That would work for __reduce__ too I think: new.instancemethod.__reduce__(C().f).
I agree! But I can't do this in this context, using __reduce__ only. In other words, I'd have to add stuff to copyreg.py, which I tried to circumvent.
Or you could change the pickling system. Your choice of what to change and what not to change seems a bit arbitrary. :-)
...
OK, so you *are* messing with internals after all (== changing C code), right? Or else how do you accomplish this?
Yessir, I'm augmenting all things-to-be-pickled with __reduce__ methods. And this time is the first time that it doesn't work.
But not necessarily the last time. :-)
...
I have a real job too, that's why I have little time to help you. :-(
I agree (and I didn't ask *you* in the first place), but still I'd like to ask the general question: Is this really the right way to handle bound objects? Is the is_data criterion correct? If I am asking for an attribute that makes *only* sense if it is bound, like in the parameter-less __reduce__ case, wouldn't it be the correct behavior to give me that bound object?
I have the strong impression that there is some difference in methods which isn't dealt with, correctly, at the moment. If a method wants to be bound to something, it should be get bound to something. Especially, if this method is useless without being bound.
It's not that it isn't being bound. It's that the *wrong* attribute is being bound (the function's __reduce__ method, bound to the function object, is returned!).
Please, swallow this idea a little bit, before rejecting it. I think that "is_data" is too rough and doesn't fit the requirements, all the time.
I agree. The bound method's attributes should always win, since bound methods only have a small, fixed number of attributes, and they are all special for bound methods. This *is* a change in functionality, even though there appear to be no unit tests for it, so I'm reluctant to fix it in 2.3. But I think in 2.4 it should definitely change. --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido van Rossum wrote:
Summary: Chistian is right after all. instancemethod_getattro should always prefer bound method attributes over function attributes.
Guido, I'm very happy with your decision, which is most probably a wise decision (without any relation to me). The point is, that I didn't know what's right or wrong, so basically I was asking for advice on a thing I felt unhappy with. So I asked you to re-think if the behavior is really what you indented, or if you just stopped, early. Thanks a lot! That's the summary and all about it, you can skip the rest if you like. ----------------------------------------------------------------------- ...
Sure I'm fiddling internaly, but simply by installing some __reduce__ methids, hoping that they work.
OK, so you *could* just make the change you want, but you are asking why it isn't like that in the first place. Good idea...
I actually hacked a special case for __reduce__, to see whether it works at all, but then asked, of course. Most of my pickling stuff might be of general interest, and changing semantics is by no means what I ever would like to do without following the main path. ...
I added __reduce__ to the PyMethod type and tried to figure out why it didn't take it.
OK. Stating that upfront would have helped...
Sorry about that. I worked too long on these issues already and had the perception that everybody knows that I'm patching __reduce__ into many objects like a bozo :-) ...
In other words, shouldn't things that are only useful as bound things, always be bound?
This question doesn't address the real issue, which is the attribute delegation to the underlying function object.
Correct, I misspelled things. Of course there is binding, but the chain back to the instance is lost. ...
The *intention* was for the 2.2 version to have the same behavior: only im_func, im_self and im_class would be handled by the bound method, other attributes would be handled by the function object.
Ooh, I begin to understand!
This is what the IsData test is attempting to do -- the im_* attributes are represented by data descriptors now. The __class__ attribute is also a data descriptor, so that C().x.__class__ gives us
rather than .
IsData is a test for having a write method, too, so we have the side effect here that im_* works like I expect, since they happen to be writable? Well, I didn't look into 2.3 for this, but in 2.2 I get
a().x.__class__=42 Traceback (most recent call last): File "<stdin>", line 1, in ? TypeError: __class__ must be set to new-style class, not 'int' object [9511 refs]
which says for sure that this is a writable property, while
a().x.im_class=42 Traceback (most recent call last): File "<stdin>", line 1, in ? TypeError: readonly attribute [9511 refs]
seems to be handled differently. I only thought of IsData in terms of accessing the getter/setter wrappers.
But for anything else, including the various methods that all objects inherit from 'object' unless they override them, the choice was made to let the function attribute win.
That's most probably right to do, since most defaults from object are probably just surrogates.
But when we look at the attributes where both function and bound method provide a value, it seems that the bound method's offering is always more useful! You've already established this for __reduce__; the same is true for __call__ and __str__, and there I stopped. (Actually, I also looked at __setattr__, where delegation to the function also seems a mistake: C().x.foo = 42 is refused, but C().x.__setattr__('foo', 42) sets the attribute on the function, because this returns the (bound) method __setattr__ on functions.)
Your examples are much better than mine.
The pickling machinery gives me an __reduce__ interface, and I'm expecting that this is able to pickle everything.
I don't think you'd have a chance of pickle classes if you only relied on __reduce__. Fortunately there are other mechanisms. :-)
I don't need to pickle classes, this works fine in most cases, and behavior can be modified by users. They can use copy_reg, and that's one of my reasons to avoid copy_reg. I want to have the basics built in, without having to import a Python module.
(I wonder if the pickling code shouldn't try to call x.__class__.__reduce__(x) rather than x.__reduce__() -- then none of these problems would have occurred... :-)
That sounds reasonable. Explicit would have been better than implicit (by hoping for the expected bound chain). __reduce__ as a class method would allow to explicitly spell that I want to reduce the instance x of class C. x.__class__.__reduce__(x) While, in contrast x.__class__.__reduce__(x.thing) would spell that I want to reduce the "thing" property of the x instance of C. While x.__class__.__reduce__(C.thing) # would be the same as C.__reduce__(C.thing) which would reduce the class method "thing" of C, or the class property of C, or whatsoever of class C. I could envision a small extension to the __reduce__ protocol, by providing an optional parameter, which would open these new ways, and all pickling questions could be solved, probably. This is so, since we can find out whether __reduce__ is a class method or not. If it is just an instance method (implictly bound), it behaves as today. If it is a class method, is takes a parameter, and then it can find out whether to pickle a class, instance, class property or an instance property. Well, I hope. The above was said while being in bed with 39° Celsius, so don't put my words on the assay-balance. [trying to use __reduce__, only]
Or you could change the pickling system. Your choice of what to change and what not to change seems a bit arbitrary. :-)
Not really. I found __reduce__ very elegant. It gave me the chance to have almost all patches in a single file, since I didn't need to patch most of the implementation files. Just adding something to the type objects was sufficient, and this keeps my workload smaller when migrating to the next Python. Until now, I only had to change traceback.c and iterator.c, since these don't export enough of their structures to patch things from outside. If at some point somebody might decide that some of this support code makes sense for the main distribution, things should of couzrse move to where they belong. Adding to copy_reg, well, I don't like to modify Python modules from C so much, and even less I like to add extra Python files to Stackless, if I can do without it. Changing the pickling engine: Well, I'm hesitant, since it has been developed so much more between 2.2 and 2.3, and I didn't get my head into that machinery, now. What I want to do at some time is to change cPickle to use a non-recursive implementation. (Ironically, the Python pickle engine *is* non-recursive, if it is run under Stackless). So, if I would hack at cPickle at all, I would probably do the big big change, and that would be too much to get done in reasonable time. That's why I decided to stay small and just chime a few __reduce__ thingies in, for the time being. Maybe this was not the best way, I don't know.
OK, so you *are* messing with internals after all (== changing C code), right? Or else how do you accomplish this?
Yessir, I'm augmenting all things-to-be-pickled with __reduce__ methods. And this time is the first time that it doesn't work.
But not necessarily the last time. :-)
Right. probably, I will get into trouble with pickling unbound class methods. Maybe I would just ignore this. Bound class methods do appear in my Tasklet system and need to get pickled. Unbound methods are much easier to avoid and probably not worth the effort. (Yes, tomorrow I will be told that it *is* :-) ...
I agree. The bound method's attributes should always win, since bound methods only have a small, fixed number of attributes, and they are all special for bound methods.
This *is* a change in functionality, even though there appear to be no unit tests for it, so I'm reluctant to fix it in 2.3. But I think in 2.4 it should definitely change.
That means, for Py 2.2 and 2.3, my current special case for __reduce__ is exactly the way to go, since it doesn't change any semantics but for __reduce__, and in 2.4 I just drop these three lines? Perfect! sincerely - chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
Guido van Rossum wrote:
Summary: Chistian is right after all. instancemethod_getattro should always prefer bound method attributes over function attributes.
[Christian]
Guido, I'm very happy with your decision, which is most probably a wise decision (without any relation to me).
The point is, that I didn't know what's right or wrong, so basically I was asking for advice on a thing I felt unhappy with. So I asked you to re-think if the behavior is really what you indented, or if you just stopped, early.
Thanks a lot!
That's the summary and all about it, you can skip the rest if you like.
Note to python-dev folks: I will make the change in 2.4. I won't backport to 2.3 unless someone can really make a case for it; it *does* change behavior. [...]
The *intention* was for the 2.2 version to have the same behavior: only im_func, im_self and im_class would be handled by the bound method, other attributes would be handled by the function object.
Ooh, I begin to understand!
This is what the IsData test is attempting to do -- the im_* attributes are represented by data descriptors now. The __class__ attribute is also a data descriptor, so that C().x.__class__ gives us
rather than . IsData is a test for having a write method, too, so we have the side effect here that im_* works like I expect, since they happen to be writable? Well, I didn't look into 2.3 for this, but in 2.2 I get
a().x.__class__=42 Traceback (most recent call last): File "<stdin>", line 1, in ? TypeError: __class__ must be set to new-style class, not 'int' object [9511 refs]
which says for sure that this is a writable property, while
a().x.im_class=42 Traceback (most recent call last): File "<stdin>", line 1, in ? TypeError: readonly attribute [9511 refs]
seems to be handled differently.
I only thought of IsData in terms of accessing the getter/setter wrappers.
It's all rather complicated. IsData only checks for the presence of a tp_descr_set method in the type struct. im_* happen to be implemented by a generic approach for defining data attributes which uses a descriptor type that has a tp_descr_set method, but its implementation looks for a "READONLY" flag. This is intentional -- in fact, having a tp_descr_set (or __set__) method that raises an error is the right way to create a read-only data attribute (at least for classes whose instances have a __dict__). [...]
I don't need to pickle classes, this works fine in most cases, and behavior can be modified by users.
Right. When you are pickling classes, you're really pickling code, not data, and that's usually not what pickling is used for. (Except in Zope 3, which can store code in the database and hence must pickle classes. But it's a lot of work, as Jeremy can testify. :-)
(I wonder if the pickling code shouldn't try to call x.__class__.__reduce__(x) rather than x.__reduce__() -- then none of these problems would have occurred... :-)
That sounds reasonable. Explicit would have been better than implicit (by hoping for the expected bound chain).
Especially since *internally* most new-style classes do this for all of the built-in operations (operations for which there is a function pointer slot in the type struct or one of its extensions). This is different from old-style classes: a classic *instance* can overload (nearly) any special method by having an instance attribute, e.g. __add__; but this is not supported for new-style instances.
__reduce__ as a class method would allow to explicitly spell that I want to reduce the instance x of class C.
x.__class__.__reduce__(x)
While, in contrast
x.__class__.__reduce__(x.thing)
would spell that I want to reduce the "thing" property of the x instance of C.
While
x.__class__.__reduce__(C.thing) # would be the same as C.__reduce__(C.thing)
which would reduce the class method "thing" of C, or the class property of C, or whatsoever of class C.
You've lost me here. How does x.__class__.__reduce__ (i.e., C.__reduce__) tell the difference between x and x.thing and C.thing???
I could envision a small extension to the __reduce__ protocol, by providing an optional parameter, which would open these new ways, and all pickling questions could be solved, probably. This is so, since we can find out whether __reduce__ is a class method or not. If it is just an instance method (implictly bound), it behaves as today. If it is a class method, is takes a parameter, and then it can find out whether to pickle a class, instance, class property or an instance property.
Well, I hope. The above was said while being in bed with 39° Celsius, so don't put my words on the assay-balance.
I sure don't understand it. If you really want this, please sit down without a fever and explain it with more examples and a clarification of what you want to change, and how. [...]
Until now, I only had to change traceback.c and iterator.c, since these don't export enough of their structures to patch things from outside. If at some point somebody might decide that some of this support code makes sense for the main distribution, things should of couzrse move to where they belong.
Do you realize that (in C code) you can always get at a type object if you can create an instance of it, and then you can patch the type object just fine? [...]
What I want to do at some time is to change cPickle to use a non-recursive implementation. (Ironically, the Python pickle engine *is* non-recursive, if it is run under Stackless). So, if I would hack at cPickle at all, I would probably do the big big change, and that would be too much to get done in reasonable time. That's why I decided to stay small and just chime a few __reduce__ thingies in, for the time being. Maybe this was not the best way, I don't know.
What's the reason for wanting to make cPickle non-recursive? [...]
Right. probably, I will get into trouble with pickling unbound class methods. Maybe I would just ignore this. Bound class methods do appear in my Tasklet system and need to get pickled. Unbound methods are much easier to avoid and probably not worth the effort. (Yes, tomorrow I will be told that it *is* :-)
Unbound methods have the same implementation as bound methods -- they have the same type, but im_self is None (NULL at the C level). So you should be able to handle this easily. (Unbound methods are not quite the same as bare functions; the latter of course are pickled by reference, like classes.) [...]
That means, for Py 2.2 and 2.3, my current special case for __reduce__ is exactly the way to go, since it doesn't change any semantics but for __reduce__, and in 2.4 I just drop these three lines? Perfect!
Right. (I'm not quite ready for the 2.4 checkin, watch the checkins list though.) --Guido van Rossum (home page: http://www.python.org/~guido/)
Hi Guido,
Guido van Rossum wrote:
Summary: Chistian is right after all. instancemethod_getattro should always prefer bound method attributes over function attributes.
...
Note to python-dev folks: I will make the change in 2.4. I won't backport to 2.3 unless someone can really make a case for it; it *does* change behavior.
...
I only thought of IsData in terms of accessing the getter/setter wrappers.
It's all rather complicated. IsData only checks for the presence of a tp_descr_set method in the type struct. im_* happen to be implemented by a generic approach for defining data attributes which uses a descriptor type that has a tp_descr_set method, but its implementation looks for a "READONLY" flag. This is intentional -- in fact, having a tp_descr_set (or __set__) method that raises an error is the right way to create a read-only data attribute (at least for classes whose instances have a __dict__).
Arghh! This is in fact harder than I was aware of. You *have* a setter, for its existance, although it won't set, for the readonly flag. Without criticism, you are for sure not finally happy with the solution, which sounds more like a working proof of concept than a real concept which you are happy to spread on the world. I'm better off to keep my hands off and not touch it now.
[...]
I don't need to pickle classes, this works fine in most cases, and behavior can be modified by users.
Right. When you are pickling classes, you're really pickling code, not data, and that's usually not what pickling is used for. (Except in Zope 3, which can store code in the database and hence must pickle classes. But it's a lot of work, as Jeremy can testify. :-)
Heh! :-) You have not seen me pickling code, while pickling frames? All kind of frames (since Stackless has many more frame types), with running code attached, together with iterators, generators, the whole catastrophe....
(I wonder if the pickling code shouldn't try to call x.__class__.__reduce__(x) rather than x.__reduce__() -- then none of these problems would have occurred... :-)
That sounds reasonable. Explicit would have been better than implicit (by hoping for the expected bound chain).
Having that said, without understanding what you meant. See below.
Especially since *internally* most new-style classes do this for all of the built-in operations (operations for which there is a function pointer slot in the type struct or one of its extensions). This is different from old-style classes: a classic *instance* can overload (nearly) any special method by having an instance attribute, e.g. __add__; but this is not supported for new-style instances.
__reduce__ as a class method would allow to explicitly spell that I want to reduce the instance x of class C.
x.__class__.__reduce__(x)
While, in contrast
x.__class__.__reduce__(x.thing)
crap. crappedi crap. *I* was lost! ...
You've lost me here. How does x.__class__.__reduce__ (i.e., C.__reduce__) tell the difference between x and x.thing and C.thing???
Nonsense.
I could envision a small extension to the __reduce__ protocol,
... Nonsense. With 39° Celsius.
I sure don't understand it. If you really want this, please sit down without a fever and explain it with more examples and a clarification of what you want to change, and how.
Reset() Revert() I got an email from Armin Rigo today, which clearly said what to do, and I did it. it works perfectly. I patches pickle.py and cPickle.c to do essentially what Armin said: """ So I'm just saying that pickle.py in wrong in just one place: reduce = getattr(obj, "__reduce__", None) if reduce: rv = reduce() should be: reduce = getattr(type(obj), "__reduce__", None) if reduce: rv = reduce(obj) """ An almost trivial change, although I also had to change copy.py, and overall I was unhappy since this extends my patch set to more than replacing python2x.dll, but I hope this will become an official patch and back-patch. [moo moo about patching almost all from outside, but iterators and tracebacks]
Do you realize that (in C code) you can always get at a type object if you can create an instance of it, and then you can patch the type object just fine?
Sure I know that. What I hate is if I have to duplicate or change data structure declarations, if I can't access them, directly. For tracebacks, I had to add a field (one reason for the non-recursive wish, below). For iterobject.c, it was clumsy, since I had to extend the existing! method table, so I had to touch the source file, anyway. (Meanwhile, I see a different way to do it, but well, it is written...) ...
What's the reason for wanting to make cPickle non-recursive?
Several reasons. For one, the same reason why I started arguing about deeply recursive destruction code, and implemented the initial elevator destructor, you remember. (trashcan) Same reason. When __del__ crashes, cPickle will crash as well. Now that I *can* pickle tracebacks and very deep recursions, I don't want them to crash. Several people asked on the main list, how to pickle deeply nested structures without crashing pickle. Well, my general answer was to rewrite pickle in a non-recursive manner. On the other hand, my implementation for tracebacks and tasklets (with large chains of frames attached) was different: In order to avoid cPickle's shortcomings of recursion, I made the tasklets produce a *list* of all related frames, instead of having them refer to each other via f_back. I did the same for tracebacks, by making the leading traceback object special, to produce a *list* of all other traceback objects in the chain. Armin once said, "rewrite the pickle code", which I'd happily do, but I do think, the above layout changes are not that bad, anyway. WHile frame chains and traceback chains are looking somewhat recursive, they aren't really. I think, they are lists/tuples by nature, and pickling them as that not only makes the result of __reduce__ more readable and usable, but the pickle is also a bit shorter than that of a deeply nested structure.
Right. probably, I will get into trouble with pickling unbound class methods.
I'm Wrong! It worked, immediately, after I understood how.
Unbound methods have the same implementation as bound methods -- they have the same type, but im_self is None (NULL at the C level). So you should be able to handle this easily. (Unbound methods are not quite the same as bare functions; the latter of course are pickled by reference, like classes.)
Yes, here we go: It was a cake walk: static PyObject * method_reduce(PyObject * m) { PyObject *tup, *name, *self_or_class; name = PyObject_GetAttrString(m, "__name__"); if (name == NULL) return NULL; self_or_class = PyMethod_GET_SELF(m); if (self_or_class == NULL) self_or_class = PyMethod_GET_CLASS(m); if (self_or_class == NULL) self_or_class = Py_None; tup = Py_BuildValue("(O(OS))", &PyMethod_Type, self_or_class, name); Py_DECREF(name); return tup; } Works perfectly. The unpickler code later does nothing at all but do the existing lookup machinery do the work. here an excerpt: if (!PyArg_ParseTuple (args, "OS", &inst, &methname)) return NULL; /* let the lookup machinery do all the work */ ret = PyObject_GetAttr(inst, methname); Perfect, whether inst is a class or an instance, it works.
That means, for Py 2.2 and 2.3, my current special case for __reduce__ is exactly the way to go, since it doesn't change any semantics but for __reduce__, and in 2.4 I just drop these three lines? Perfect!
Dropped it, dropped it! Yay!
Right. (I'm not quite ready for the 2.4 checkin, watch the checkins list though.)
Well, after Armin's input, I dropped my special case, and instead I will submit a patch for 2.2 and 2.3, which uses your proposed way to use __reduce__ from pickle and copy. This is completely compatible and does what we want! ciao - chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
Arghh! This is in fact harder than I was aware of. You *have* a setter, for its existance, although it won't set, for the readonly flag. Without criticism, you are for sure not finally happy with the solution, which sounds more like a working proof of concept than a real concept which you are happy to spread on the world. I'm better off to keep my hands off and not touch it now.
Actually, I like it fine. There really are four categories: 0) not a descriptor 1) overridable descriptor (used for methods) 2a) read-only non-overridable descriptor (used for read-only data) 2b) writable non-overridable descriptor (used for writable data) Case (0) is recognized by not having __get__ at all. Case (1) has __get__ but not __set__. Cases (2a) and (2b) have __get__ and __set__; case (2a) has a __set__ that raises an exception. There are other (older) examples of __setattr__ implementations that always raise an exception.
I patches pickle.py and cPickle.c to do essentially what Armin said: """ So I'm just saying that pickle.py in wrong in just one place:
reduce = getattr(obj, "__reduce__", None) if reduce: rv = reduce()
should be:
reduce = getattr(type(obj), "__reduce__", None) if reduce: rv = reduce(obj) """
Right. (That's what I was trying to say, too. :-)
An almost trivial change, although I also had to change copy.py, and overall I was unhappy since this extends my patch set to more than replacing python2x.dll, but I hope this will become an official patch and back-patch.
Give it to me baby. (On SF. :-)
What's the reason for wanting to make cPickle non-recursive?
Several reasons. For one, the same reason why I started arguing about deeply recursive destruction code, and implemented the initial elevator destructor, you remember. (trashcan)
Yeah. Maybe I should get out of the serious language implementation business, because I still liked it better before. It may work, but it is incredibly ugly, and also had bugs for the longest time (and those bugs were a lot harder to track down than the bug it was trying to fix). With Tim's version I can live with it -- but I just don't like this kind of complexification of the implementation, even if it works better.
Same reason. When __del__ crashes, cPickle will crash as well.
Please don't call it __del__. __del__ is a user-level finalization callback with very specific properties and problems. You were referring to tp_dealloc, which has different issues.
Now that I *can* pickle tracebacks and very deep recursions, I don't want them to crash.
Several people asked on the main list, how to pickle deeply nested structures without crashing pickle. Well, my general answer was to rewrite pickle in a non-recursive manner.
I guess it's my anti-Scheme attitude. I just think the problem is in the deeply nested structures. There usually is a less nested data structure that doesn't have the problem. But I'll shut up, because this rant is not productive. :-(
On the other hand, my implementation for tracebacks and tasklets (with large chains of frames attached) was different: In order to avoid cPickle's shortcomings of recursion, I made the tasklets produce a *list* of all related frames, instead of having them refer to each other via f_back. I did the same for tracebacks, by making the leading traceback object special, to produce a *list* of all other traceback objects in the chain.
Hey, just what I said. :-)
Armin once said, "rewrite the pickle code", which I'd happily do, but I do think, the above layout changes are not that bad, anyway. WHile frame chains and traceback chains are looking somewhat recursive, they aren't really. I think, they are lists/tuples by nature, and pickling them as that not only makes the result of __reduce__ more readable and usable, but the pickle is also a bit shorter than that of a deeply nested structure.
Well, unclear. Frame chains make sense as chains because they are reference-counted individually.
Well, after Armin's input, I dropped my special case, and instead I will submit a patch for 2.2 and 2.3, which uses your proposed way to use __reduce__ from pickle and copy. This is completely compatible and does what we want!
Wonderful! Please send me the SF issue, I don't subscribe to SF any more. (I've done my checkin in case you wondered.) --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido says:
I guess it's my anti-Scheme attitude. I just think the problem is in the deeply nested structures. There usually is a less nested data structure that doesn't have the problem.
and then he says:
Well, unclear. Frame chains make sense as chains because they are reference-counted individually.
which surely goes to show that sometimes it *does* make sense to use a deeply nested structure? Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
Greg Ewing wrote:
Guido says:
I guess it's my anti-Scheme attitude. I just think the problem is in the deeply nested structures. There usually is a less nested data structure that doesn't have the problem.
and then he says:
Well, unclear. Frame chains make sense as chains because they are reference-counted individually.
which surely goes to show that sometimes it *does* make sense to use a deeply nested structure?
You might interpret him this way. But I don't think he had my implementation of frame chain pickling in mind, because he doesn't know it, and nobody but me probably has a working one. I'm pickling disjoint frame chains, and in my case, these are linked in both directions, via f_back, and via f_callee, for other reasons. There is no reason for nested pickling, just because of the caller/callee relationship. I agree there might be useful situations for deeply nested structures, but not this one. Instead, it would be asking for problems. ciao - chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
Guido says:
I guess it's my anti-Scheme attitude. I just think the problem is in the deeply nested structures. There usually is a less nested data structure that doesn't have the problem.
and then he says:
Well, unclear. Frame chains make sense as chains because they are reference-counted individually.
which surely goes to show that sometimes it *does* make sense to use a deeply nested structure?
Well, without deeply nested data structures the stack wouldn't be that deep, would it? :-) --Guido van Rossum (home page: http://www.python.org/~guido/)
[Christian]
Several people asked on the main list, how to pickle deeply nested structures without crashing pickle. Well, my general answer was to rewrite pickle in a non-recursive manner.
[Guido]
I guess it's my anti-Scheme attitude. I just think the problem is in the deeply nested structures. There usually is a less nested data structure that doesn't have the problem. But I'll shut up, because this rant is not productive. :-(
Ya, but it *used* to be -- in the early days, many people learned a lot about writing better programs by avoiding constructs Python penalized (nested functions, cyclic references, deep recursion, very long reference chains, massively incestuous multiple inheritance). Learning to design with flatter data structures and flatter code was highly educational, and rewarding, at least for those who played along. I suppose that's gone for good now. An irony specific to pickle is that cPickle coding was driven mostly by Zope's needs, and multi-gigabyte Zope databases live happily with its recursive design -- most data ends up in BTrees, and those hardly ever go deeper than 3 levels. I don't think it's coincidence that, needing to find a scalable container type with demanding size and speed constraints, Jim ended up with a "shallow" BTree design. The lack of need for deep C recursion was then a consequence of needing to avoid (for search speed) long paths from root to data. Oh well. The next generation will learn the hard way <wink>. looking-forward-to-death-ly y'rs - tim
[Guido]
I guess it's my anti-Scheme attitude. I just think the problem is in the deeply nested structures. There usually is a less nested data structure that doesn't have the problem.
A couple more thoughts: There's a difference between nested data structures and recursion. Use of one doesn't necessarily imply the other. Also, whether a given data structure is "nested" or not can depend on your point of view. Most people wouldn't consider a linked list to be nested -- it may be "wide", but it's not usually thought of as "deep". I don't think it's unreasonable to ask for a pickle that doesn't use up a recursion level for each unit of width in such a structure. Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
Guido van Rossum wrote: Hi Guido, this is a little follow-up. I didn't forget about it! ...
So I'm just saying that pickle.py in wrong in just one place:
reduce = getattr(obj, "__reduce__", None) if reduce: rv = reduce()
should be:
reduce = getattr(type(obj), "__reduce__", None) if reduce: rv = reduce(obj) """
Right. (That's what I was trying to say, too. :-)
An almost trivial change, although I also had to change copy.py, and overall I was unhappy since this extends my patch set to more than replacing python2x.dll, but I hope this will become an official patch and back-patch.
Give it to me baby. (On SF. :-)
So here the bad news: After changing the C code to use the new style, and also changing all occourances in Python code, I used this on Zope, and ExtensionClass did not play well, it cannot stand this protocol unchanged. Zope won't start. Therefore, I reverted my changes and now I'm waiting for the day where ExtensionClass is no loger used. (or should I patch *that* complicated stuff as well?) cheers - chris -- Christian Tismer :^) mailto:tismer@tismer.com Mission Impossible 5oftware : Have a break! Take a ride on Python's Johannes-Niemeyer-Weg 9a : *Starship* http://starship.python.net/ 14109 Berlin : PGP key -> http://wwwkeys.pgp.net/ work +49 30 89 09 53 34 home +49 30 802 86 56 mobile +49 173 24 18 776 PGP 0x57F3BF04 9064 F4E1 D754 C2FF 1619 305B C09C 5A3B 57F3 BF04 whom do you want to sponsor today? http://www.stackless.com/
So I'm just saying that pickle.py in wrong in just one place:
reduce = getattr(obj, "__reduce__", None) if reduce: rv = reduce()
should be:
reduce = getattr(type(obj), "__reduce__", None) if reduce: rv = reduce(obj) """
Right. (That's what I was trying to say, too. :-)
An almost trivial change, although I also had to change copy.py, and overall I was unhappy since this extends my patch set to more than replacing python2x.dll, but I hope this will become an official patch and back-patch.
Give it to me baby. (On SF. :-)
So here the bad news: After changing the C code to use the new style, and also changing all occourances in Python code, I used this on Zope, and ExtensionClass did not play well, it cannot stand this protocol unchanged. Zope won't start.
Therefore, I reverted my changes and now I'm waiting for the day where ExtensionClass is no loger used. (or should I patch *that* complicated stuff as well?)
No, just wait until it dies. I think Jim wants it to die to; the latest Zope version requires Python 2.3.2 but it will take time to retire it, and old Zope versions are persistent. --Guido van Rossum (home page: http://www.python.org/~guido/)
participants (5)
-
Christian Tismer -
David Ascher -
Greg Ewing -
Guido van Rossum -
Tim Peters