[Python-Dev] instancemethod_getattro seems to be partially wrong

Christian Tismer tismer at tismer.com
Sun Nov 23 00:33:48 EST 2003


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 at 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/





More information about the Python-Dev mailing list