Re: TreeVocabulary in zope.schema.vocabulary

On Wed, Jan 25, 2012 at 01:55:28AM +0200, Jan-Carel Brand wrote:
> 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:
> > > 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.
<...>
> > > 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 :)

For extra fun: one is an alias for the other in newer ZODB versions.

> > > > > 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

Yes.  And if I pass an OrderedDict to your .fromDict(), it will be
discarded and all the items inserted into a regular dict, forgetting
their original order.

But anyway, I'm fine with you saying "explicit ordering is not
supported; it's up to the widget to sort each tree level appropriately".
In the class docstring, say.  ;-)

> > 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.

I've no objections remaining (other than that little thing about
explicit ordering).

Marius Gedminas
-- 
http://pov.lt/ -- Zope 3/BlueBream consulting and development

_______________________________________________
Zope-Dev maillist  -  [email protected]
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 )

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iEYEARECAAYFAk8f8hQACgkQkVdEXeem14/gTQCfXlY3stcf84wVAkUb+QwkOdX/
SfsAnRDeLC6rrevQc6X5nD5CA5OFbm51
=1bZm
-----END PGP SIGNATURE-----