Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Definition of externalizable-by-default sequences is too narrow. #122

Open
jamadden opened this issue Oct 15, 2021 · 0 comments
Open

Definition of externalizable-by-default sequences is too narrow. #122

jamadden opened this issue Oct 15, 2021 · 0 comments

Comments

@jamadden
Copy link
Member

If there is nothing specifically registered to externalize an object, and the object itself doesn't have a usable toExternalObject() method, we go through a set of other steps to try to externalize it. One of the last is recoginizing something as a sequence, which we will externalize as a LocatedExternalList:

https://github.com/NextThought/nti.externalization/blob/59896bebe27a8289036c634ca5f1b12c66e57faf/src/nti/externalization/externalization/externalizer.py#L281-L291

That checks for the (old) zope.interface.common.sequence.IFiniteSequence interface, or a specific list of types:

https://github.com/NextThought/nti.externalization/blob/59896bebe27a8289036c634ca5f1b12c66e57faf/src/nti/externalization/externalization/externalizer.py#L70-L80

This can surprise people that write sequence-like objects, especially if they extend collections.abc.Sequence: they don't get externalized, and the reason isn't obvious (especially if the sequence-like object is just replacing a plain list or tuple). The solution right now is to declare the type as @implementer(IFiniteSequence) but that shouldn't be necessary.

We could either add collections.abc.Sequence to that list (but checking for abstract types is relatively slow, and isinstance is O(n) in the list of types), or we could take advantage of the new-and-improved ABC interfaces in zope.interface:

>>> from zope.interface.common.collections import ISequence
>>> ISequence.__sro__
(<ABCInterfaceClass zope.interface.common.collections.ISequence>,
 <ABCInterfaceClass zope.interface.common.collections.IReversible>,
 <ABCInterfaceClass zope.interface.common.collections.ICollection>,
 <ABCInterfaceClass zope.interface.common.collections.ISized>,
 <ABCInterfaceClass zope.interface.common.collections.IIterable>,
 <ABCInterfaceClass zope.interface.common.collections.IContainer>,
 <ABCInterfaceClass zope.interface.common.ABCInterface>,
 <InterfaceClass zope.interface.Interface>)
>>> ISequence.providedBy([])
True
>>> from persistent.list import PersistentList
>>> ISequence.providedBy(PersistentList())
True
>>> ISequence.providedBy(())
True

Unfortunately, that doesn't automatically pick up new subclasses of Sequence defined after ISequence was imported, so that's not a complete solution currently:

>>> from collections.abc import Sequence
>>> class S(Sequence):
...     __getitem__ = __len__ = lambda self: None
...
>>> ISequence.providedBy(S())
False
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant