On Wed, 2012-01-25 at 00:52 +0200, Marius Gedminas wrote:
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...
Yeah, using PersistentMapping was a mistake, firstly because persistence is not necessary and secondly because it introduces a dependency on Persistence.
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? ;)
It was first the one, and then the other :)
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.
Python 2.7 has an OrderedDict class in the collections module: http://docs.python.org/dev/whatsnew/2.7.html#pep-0372
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.
Thanks for the suggestion, I did that. JC