Support for iterable __slots__ #67

Closed
jaraco opened this Issue Mar 14, 2014 · 6 comments

Comments

Projects
None yet
3 participants
@jaraco
Contributor

jaraco commented Mar 14, 2014

I've discovered recently that Python allows __slots__ to be an arbitrary iterable. Consider:

>>> class X:
>>>   __slots__ = iter('abc')
>>> x = X()
>>> print(list(x.__slots__))
[]

ugh.

This outcome is a problem for jsonpickle because it relies on __slots__ to serialize the object.

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Aug 21, 2014

Contributor

It seems that the stdlib pickle module doesn't rely on __slots__ (there's no mention of it in the pickle module itself), but the code does raise an error when one tries to pickle a class with slots:

>>> x.__reduce__()
Traceback (most recent call last):
  File "c:\python\lib\copyreg.py", line 69, in _reduce_ex
    getstate = self.__getstate__
AttributeError: 'X' object has no attribute '__getstate__'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\python\lib\copyreg.py", line 72, in _reduce_ex
    raise TypeError("a class that defines __slots__ without "
TypeError: a class that defines __slots__ without defining __getstate__ cannot be pickled

This behavior suggests to me that jsonpickle should also raise a similar error (rather than silently omitting the attributes, which is the current behavior).

>>> x.a = 3
>>> jsonpickle.encode(x)
'{"py/object": "__main__.X"}'

I wish when I'd filed this ticket I had made a note of where I encountered this situation in the real world.

Contributor

jaraco commented Aug 21, 2014

It seems that the stdlib pickle module doesn't rely on __slots__ (there's no mention of it in the pickle module itself), but the code does raise an error when one tries to pickle a class with slots:

>>> x.__reduce__()
Traceback (most recent call last):
  File "c:\python\lib\copyreg.py", line 69, in _reduce_ex
    getstate = self.__getstate__
AttributeError: 'X' object has no attribute '__getstate__'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\python\lib\copyreg.py", line 72, in _reduce_ex
    raise TypeError("a class that defines __slots__ without "
TypeError: a class that defines __slots__ without defining __getstate__ cannot be pickled

This behavior suggests to me that jsonpickle should also raise a similar error (rather than silently omitting the attributes, which is the current behavior).

>>> x.a = 3
>>> jsonpickle.encode(x)
'{"py/object": "__main__.X"}'

I wish when I'd filed this ticket I had made a note of where I encountered this situation in the real world.

@marcintustin

This comment has been minimized.

Show comment
Hide comment
@marcintustin

marcintustin Aug 22, 2014

Contributor

I've been adding getstate/setstate support. See #81

I mention this because I've been adding a bunch of unittests to verify Protocol 2 compatibility if you want to add some for this, go ahead. This seems like an opportune time to address this issue as well.

Incidentally, I'm holding of development until my current pull request is merged or I get some feedback on whether it's the right thing.

@davvid @jaraco

Contributor

marcintustin commented Aug 22, 2014

I've been adding getstate/setstate support. See #81

I mention this because I've been adding a bunch of unittests to verify Protocol 2 compatibility if you want to add some for this, go ahead. This seems like an opportune time to address this issue as well.

Incidentally, I'm holding of development until my current pull request is merged or I get some feedback on whether it's the right thing.

@davvid @jaraco

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Aug 23, 2014

Member

I would not mind if we went above and beyond stdlib pickle module. It doesn't handle slots, but we do support regular __slots__ = ('foo',) so it shouldn't be to hard to support arbitrary iterable slots.

Member

davvid commented Aug 23, 2014

I would not mind if we went above and beyond stdlib pickle module. It doesn't handle slots, but we do support regular __slots__ = ('foo',) so it shouldn't be to hard to support arbitrary iterable slots.

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Aug 23, 2014

Contributor

I'm not sure it's possible to support arbitrary iterable slots because by the time our code reaches the object, the __slots__ has already been consumed by the class constructor, so list(Class.__slots__) is always [].

Contributor

jaraco commented Aug 23, 2014

I'm not sure it's possible to support arbitrary iterable slots because by the time our code reaches the object, the __slots__ has already been consumed by the class constructor, so list(Class.__slots__) is always [].

@davvid davvid closed this in 9c5cbaa Aug 23, 2014

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Aug 23, 2014

Member

@jaraco I reached the same conclusion, but then I put in a fallback where we try dir() with a filtered set of names. It's kinda hacky so I wouldn't be against dropping the feature, but it does work "most of the time". It's certainly an edge case.

Member

davvid commented Aug 23, 2014

@jaraco I reached the same conclusion, but then I put in a fallback where we try dir() with a filtered set of names. It's kinda hacky so I wouldn't be against dropping the feature, but it does work "most of the time". It's certainly an edge case.

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Aug 23, 2014

Contributor

Awesome. Well done. And thanks.

Contributor

jaraco commented Aug 23, 2014

Awesome. Well done. And thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment