Hi Charlie On Tue, 2012-01-24 at 15:06 +0100, Charlie Clark wrote:
Hiya,
Am 24.01.2012, 12:35 Uhr, schrieb Jan-Carel Brand <lists@opkode.com>:
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've only glanced cursorily at the source but I'm not sure that a merge is okay yet.
Yes, Marius pointed that out as well.
At least not all of the methods have doc strings and one of them seems to have a doctest.
I've clarified some of the docstrings and added the missing one. None have doctests, perhaps you are referring to fromDict, which gives an example dict to show the required structure. I guess that could easily be turned into a doctest, I'll look into it.
It would be nice to expand the README here.
I don't see anything about vocabs there at all, but I'm willing to add some tests.
getTerm is a copy of the SimpleVocabulary method. Using the @classmethod decorator is fine but inconsistent with other vocabulary classes. Using the decorator is more readable so I'd suggest changing all relevant methods to do that.
Ok, I changed all the other methods to also use the decorator.
Off the top of my head I don't know which Python versions support classmethods. That should be checked in case zope.schema is in a ZTK that is running on Python 2.4
Doesn't seem to be a problem for Python 2.4: http://www.python.org/dev/peps/pep-0318/#current-syntax
I would just like someone to sign it off. Should I rather create a ticket on launchpad? Or otherwise, should I just go ahead and merge the changes to trunk?
I would suggest going via launchpad if only because it is a better paper trail.
Ok.
At the moment I'm still not quite sure whether this is required in zope.schema.
I think that a zope3 style TreeVocabulary is indeed needed. We use them quite a bit in Plone (currently only for Archetypes). If not in zope.schema, then in a separate egg?
Do widgets only exist for z3c.form?
Well, there wasn't such a widget for z3c.form. The dynatree widget was for Archetypes. I (and Johan Beyers) ported it to z3c.form. Thanks for taking the time. JC