[Zope-dev] TreeVocabulary in zope.schema.vocabulary

[Jan-Carel Brand]

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