__setattr__ in subclasses of pure-Python Persistent can throw AttributeError in cases the C version does not

[jamadden]

We have some code that subclasses Persistent and modifies _p_changed handling, something like this:

class X(Persistent):
    def __setattr__(self, name, value):
        was_changed = self._p_changed
        super(X,self).__setattr__(name, value)
        if not was_changed and some_magic_reason():
            # Actually, we're not changed. Don't try to persist this.
            self._p_changed = False

This works fine if we get the C version of Persistent, but if we get the Python version it blows up due to the way Persistent.__new__ and Persistent._get_changed interact with __setattr__:

>>> X()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "persistent/persistence.py", line 56, in __new__
    inst.__jar = None
  File "<console>", line 3, in __setattr__
  File "persistent/persistence.py", line 222, in __getattribute__
    return _OGA(self, name)
  File "persistent/persistence.py", line 118, in _get_changed
    if self.__jar is None:
  File "persistent/persistence.py", line 222, in __getattribute__
    return _OGA(self, name)
AttributeError: _Persistent__jar

Is this a bug in the Python version? It is a behaviour difference from the C version. If it's not a bug, is there a suggested workaround? We can alter our code to not read _p_changed if __setattr__ is called with a private property (name.startswith('_Persistent')), it just seems a bit unclean to rely on implementation details like that. (Using Persistent._p_setattr doesn't work.)

If it is a bug, I'd be happy to submit a PR to test and fix it. I would probably make __new__ call object.__setattr__, bypassing subclasses altogether, unless there are other suggestions.

[jamadden]

I went ahead and attached a proposed solution for review.

[tseaver]

Looks good, thank you!