On Tue, Jan 24, 2012 at 07:34:03PM +0200, Jan-Carel Brand wrote:
Missing tests: by inheriting from SimpleVocabulary you also gain .fromItems() and .fromValues(). Do those work? They pass a list of terms to __init__, which seems to expect a dict now. Override and add a raise NotImplementedError? Or just make them work?
I now subclass PersistentMapping instead of SimpleVocabulary, so this is not an issue anymore.
Ok. But why Persistent? None of the other vocabularies are persistent...
What's the use case for a tree vocabulary? A widget that displays the tree structure explicitly?
Yes. In my case, it's for the widget in collective.dynatree. This is a fairly common use-case in Plone. Products.ATVocabularyManager also has hierarchical vocabularies.
*nod*
It seems... difficult to extract that tree structure using just the public API. Actually, it's impossible: __iter__ doesn't return all the terms, just top-level ones. Am I missing something?
I've changed the TreeVocabulary to subclass from PersistentDict. So the vocabulary itself now acts as a dict.
So is it PersistentMapping or PersistentDict then? ;)
Perhaps I should rephrase :)
I would like my changes to be merged with the zope.schema trunk. The tests I've added provide 100% coverage of the TreeVocabulary code.
I would just like someone to sign it off.
-1 because of the concerns above.
Fair enough. Have your concerns been addressed properly?
Thank you, yes. I'm still wondering about the possibility of ordered trees. And I'm -1 for subclassing PersistentMapping. It may tempt people into storing tree vocabularies in the ZODB, and then maybe even modifying them. And you have plenty of non-persistent dicts in the internal structure. I think it would be better to subclass a regular dict, and document that you ITreeVocabulary is a dict-like object by making it inherit IEnumerableMapping. Regards, Marius Gedminas -- http://pov.lt/ -- Zope 3/BlueBream consulting and development