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

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Zope-Dev maillist  -  Zope-Dev@zope.org
https://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists -
 https://mail.zope.org/mailman/listinfo/zope-announce
 https://mail.zope.org/mailman/listinfo/zope )

Reply via email to