`_p_repr` drops Acquisition wrappers, even when found on the instance, with the C extensions about persistent HOT 7 OPEN

Sorry for coming in late on this.

What is rational for _p_repr? This seems to add needless complexity. Why not let subclasses override __repr__ if they want to override __repr__?

Is there some compelling use case here?

Would the crazyness around acquisition go away if we didn't do the _p_repr dance?

from persistent.

_p_repr was originally proposed back in #11 by @jimfulton. I believe it allows subclasses to define a representation for the common case without having to worry about error handling for the uncommon case that the connection has been closed or they otherwise cannot access their persistent state (that's a real problem; my group had defined a workaround).

Overriding __repr__ is a viable option, just as it always has been. It works fine in the case of acquisition, as the Zope/Plone projects have had to demonstrate. Really, _p_repr doesn't enter the conversation there, except that it would be really nice if the benefits of _p_repr also happened to work in the case of acquisition. It currently doesn't. I'm not sure there's a way to make it do so (perhaps something in Persistence? I haven't thought that much about it).

This only comes up because the improved __repr__ in persistent.Persistent happened to change the MRO of __repr__ in some complex multiple-inheritance classes in Plone; introducing a _p_repr method (which didn't suffer from multiple inheritance) in a base class could have easily solved the doctest problem in a simple way, but because it doesn't handle acquisition, that didn't work.

from persistent.

Ha ha, Ah, right, dealing with closed connections... I vaguely remember this being a problem. :)

from persistent.

I'm very confused. Is something actually defining _p_repr and is the problem that this isn't woring as expected?

from persistent.

Was this working and broke with some recent PR?

from persistent.

Acquisition was working for some Zope objects' __repr__. Because of the complicated way that extension classes are added to the MRO, adding __repr__ to persistent.Persistent unexpectedly overrode __repr__ for those objects, and the nice workaround of adding a _p_repr to the base class that defined __repr__ didn't work with acquisition. The only solution we found was to introduce a mixin class that implements __repr__ and explicitly put it at the front of the MRO for the affected classes.

from persistent.

I'll play with this a bit today.

Much thanks for your work on this BTW.

from persistent.