Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hiya, ... lots cut ... I like the _populateIndexes() method. Having a single pass without a signifying parameter makes it easier to understand. Am 06.02.2012, 10:12 Uhr, schrieb Jan-Carel Brand li...@opkode.com: A vocabulary must minimally be able to determine whether it contains a value, to create a term object for a value, and to return a query interface (or None) to find items in itself. So it looks like someone thinks that any vocabulary must be able to create a term object. Thoughts on that? Yes: I think it's much more important to define what kind of terms are expected. Apart from local utilities I've never come across the need to add terms individually and even then term generation is outside the scope of the vocabulary as you already have a term factory. Validating terms seems more important so I guess and __setitem__() method which imposes the same checks as you run in _populateIndexes() might be worthwhile to stop the index being fouled up by application code. I think I'm now finished and have implemented everybody's suggestions and improvements. The TreeVocabulary now has a terms_factory attribute which is an OrderedDict by default and which can be overridden in a subclass to an alternative datastructure. It also implements IEnumerableMapping. All the TreeVocabulary methods are covered by tests and I tested with Python 2.6 and Python 2.7. If there aren't any more objections, I'd like to now merge with trunk. I think it's okay to go ahead and merge. Charlie -- Charlie Clark Managing Director Clark Consulting Research German Office Kronenstr. 27a Düsseldorf D- 40217 Tel: +49-211-600-3657 Mobile: +49-178-782-6226 ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On Mon, 2012-01-30 at 15:42 +0100, Charlie Clark wrote: Am 30.01.2012, 14:33 Uhr, schrieb Jan-Carel Brand li...@opkode.com: ... lots cut ... Yes, the values must be unique, because we do value lookups. The title attr doesn't have to be unique though. You could have a Forestry department for two different regions. The title will be the same, but the value and token attrs can't. I think the tests should be extended to show that this also includes nesting because this is non-obvious. ie. I believe that the region Izmir is within the state of Izimir in Turkey. Ok, I've added the method *test_nonunique_values_and_tokens* to the tests. This should be possible by calling _getPathToTreeNode during one of the passes through _flattenTree. getTermPath would then just need to do a lookup on this. I don't like the way the path_node gets implicitly populated during a call to _flattenTree. hm, okay. Personally, I think you should be able to populate your dictionaries with only a single pass through the terms. However, as this only needs to happen when the application starts we don't need to worry too much about the performance. I think you're right, so I've refactored _flattenTree and _getPathIndex into a single method _populateIndexes that populates all three indexes with one tree-traversal. I'd rather have a separate method that calculates the path and then explicitly assign it to self.path_node. In any case, there is now a node_index in the code snip but I don't see the advantage of cls.createTerm(*args) over SimpleTerm(*args) See above. createTerm is there to let developers override it and provide their own term objects. Do you have a concrete use case for this? Not really, but that doesn't mean it doesn't exist. Then someone will speak up for it if they need it or do their own subclassing/composition as required. Otherwise it's just food for warts. Remember that createTerm is a convenience method only. Frankly, I don't see the need for it in what is a fairly specialised class. I like consistency and symmetry, so if SimpleVocabulary has it, as an add-on developer I'd expect for TreeVocabulary to also have it. I don't however feel very strongly about it though, and I wanna get this done, so I removed it. Well, we could always think about removing it from SimpleVocabulary: it's not in the interface so no subclass actually has the right to depend on it. ;-) Looking at the documentation in fields.txt I see the following: A vocabulary must minimally be able to determine whether it contains a value, to create a term object for a value, and to return a query interface (or None) to find items in itself. So it looks like someone thinks that any vocabulary must be able to create a term object. Thoughts on that? I think I'm now finished and have implemented everybody's suggestions and improvements. The TreeVocabulary now has a terms_factory attribute which is an OrderedDict by default and which can be overridden in a subclass to an alternative datastructure. It also implements IEnumerableMapping. All the TreeVocabulary methods are covered by tests and I tested with Python 2.6 and Python 2.7. If there aren't any more objections, I'd like to now merge with trunk. Regards J-C ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On Sun, 2012-01-29 at 16:17 +0100, Charlie Clark wrote: snip From the zope.schema 4.0.0 release notes: Port to Python 3. This adds a dependency on six and removes support for Python 2.5 Shouldn't that also mean a preference of for key in _dict over for key in _dict.keys() ? Though you might prefer .items() in _getPathToTreeNode() i.e def _getPathToTreeNode(self, tree, node) path = [] for parent, child in tree.items(): if node == parent.value: return [node] path = self._getPathToTreeNode(child, node) if path: path.insert(0, parent.value) break return path Thanks, I've taken this code from the diff file you gave. So if we want to use OrderedDict (which is from Python = 2.7), we just need to bridge the Python 2.6 version with the ordereddict package: http://pypi.python.org/pypi/ordereddict/1.1 This would introduce a new dependency for zope.schema on Python 2.6, I don't know if that's acceptable or not. I think it's perfectly justified in this case and similar to what has happened with other libraries like ElementTree in the past that have made life easier and subsequently been adopted by the standard library. Marius and Souheil, what do you think about this? I think it probably still makes sense to have a dict_factory as Souheil suggested, but then we make the default type OrderedDict and not dict. Any objections? In setup.py one could specify the extra dependency only for Python 2.7: import sys REQUIRES = ['setuptools', 'zope.interface = 3.6.0', 'zope.event', 'six', ] if sys.version_info (2 , 7): REQUIRES += ['ordereddict'], setup( # [...] install_requires=REQUIRES, # [...] Yep. snip I however don't see how one could use a generator for the recursive methods that return dictionaries. With regards to the recurse method (now called _getPathToTreeNode), I don't see how one could use a generator in a more efficient manner than the normal recursion that's being used currently. I played around with it and the best I could come up with is this: def generator(_dict, value, path=[]): if value in _dict.keys(): yield path+[value] for key in _dict.keys(): for path in recurse(_dict[key], value, path+[key]): if value in path: yield path You still have to recurse through the different branches until you find the node that you are looking for and you still need to store the path in a list. So what would be the added value? What's more, the generator returns a list within a list, like so: [['Regions', 'Germany', 'Bavaria']], which I find clunky. You're probably right. I think I was getting ahead of myself wondering about possible issues with deeply nested vocabularies. Any real improvement would probably involve a node index. I notice that the examples do not allow for departments in regions to have the same name as the region (common enough in some countries) so you could simply add the index keyed by node value with the path as the value when the class is created. Yes, the values must be unique, because we do value lookups. The title attr doesn't have to be unique though. You could have a Forestry department for two different regions. The title will be the same, but the value and token attrs can't. This should be possible by calling _getPathToTreeNode during one of the passes through _flattenTree. getTermPath would then just need to do a lookup on this. I don't like the way the path_node gets implicitly populated during a call to _flattenTree. I'd rather have a separate method that calculates the path and then explicitly assign it to self.path_node. In any case, there is now a node_index in the code :) snip but I don't see the advantage of cls.createTerm(*args) over SimpleTerm(*args) See above. createTerm is there to let developers override it and provide their own term objects. Do you have a concrete use case for this? Not really, but that doesn't mean it doesn't exist. Remember that createTerm is a convenience method only. Frankly, I don't see the need for it in what is a fairly specialised class. I like consistency and symmetry, so if SimpleVocabulary has it, as an add-on developer I'd expect for TreeVocabulary to also have it. I don't however feel very strongly about it though, and I wanna get this done, so I removed it. J-C ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On Fri, 2012-01-27 at 11:50 +0100, Souheil CHELFOUH wrote: Yes, Marius got exactly what I meant, this is how I usually make the not-so-pluggable stuff pluggable :) About the fact to release it out of zope.schema, it's also to be able to make it live a bit and prove itself. Then, including it and changing the API can be easily justified. I'm not afraid of the changes in zope.schema. z3c.form is not the only form option, I'll probably use it in dolmen.forms that came from zeam.form. So, whatever you decide, we'll try to make it easy for you to contribute. It tends to get to complicated to do anything in zope, lately :) Ok, I've implemented the idea of the dict_factory, making sure to implement all the methods required by IEnumerableMapping and IReadMapping. All tests pass and it works in my specific end use-case. However, since we can use the OrderedDict type in Python 2.7 by means of the ordereddict package, what do you think of making that the default dict type and adding an 'ordereddict' dependency for older Python versions? (Please see Charlie and my discussion about it.) If we still use the concept of a dict_factory, it wouldn't make sense to implement the OrderedDict specific methods like popItem, but the keys method would be ordered, which I think would be enough. J-C ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hiya, Am 30.01.2012, 14:33 Uhr, schrieb Jan-Carel Brand li...@opkode.com: ... lots cut ... Yes, the values must be unique, because we do value lookups. The title attr doesn't have to be unique though. You could have a Forestry department for two different regions. The title will be the same, but the value and token attrs can't. I think the tests should be extended to show that this also includes nesting because this is non-obvious. ie. I believe that the region Izmir is within the state of Izimir in Turkey. This should be possible by calling _getPathToTreeNode during one of the passes through _flattenTree. getTermPath would then just need to do a lookup on this. I don't like the way the path_node gets implicitly populated during a call to _flattenTree. hm, okay. Personally, I think you should be able to populate your dictionaries with only a single pass through the terms. However, as this only needs to happen when the application starts we don't need to worry too much about the performance. I'd rather have a separate method that calculates the path and then explicitly assign it to self.path_node. In any case, there is now a node_index in the code snip but I don't see the advantage of cls.createTerm(*args) over SimpleTerm(*args) See above. createTerm is there to let developers override it and provide their own term objects. Do you have a concrete use case for this? Not really, but that doesn't mean it doesn't exist. Then someone will speak up for it if they need it or do their own subclassing/composition as required. Otherwise it's just food for warts. Remember that createTerm is a convenience method only. Frankly, I don't see the need for it in what is a fairly specialised class. I like consistency and symmetry, so if SimpleVocabulary has it, as an add-on developer I'd expect for TreeVocabulary to also have it. I don't however feel very strongly about it though, and I wanna get this done, so I removed it. Well, we could always think about removing it from SimpleVocabulary: it's not in the interface so no subclass actually has the right to depend on it. ;-) Charlie -- Charlie Clark Managing Director Clark Consulting Research German Office Kronenstr. 27a Düsseldorf D- 40217 Tel: +49-211-600-3657 Mobile: +49-178-782-6226 ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Snip snip Marius and Souheil, what do you think about this? I think it probably still makes sense to have a dict_factory as Souheil suggested, but then we make the default type OrderedDict and not dict. This sounds perfectly reasonnable. Any objections? Not really ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Yes, Marius got exactly what I meant, this is how I usually make the not-so-pluggable stuff pluggable :) About the fact to release it out of zope.schema, it's also to be able to make it live a bit and prove itself. Then, including it and changing the API can be easily justified. I'm not afraid of the changes in zope.schema. z3c.form is not the only form option, I'll probably use it in dolmen.forms that came from zeam.form. So, whatever you decide, we'll try to make it easy for you to contribute. It tends to get to complicated to do anything in zope, lately :) 2012/1/26 Marius Gedminas mar...@gedmin.as: On Thu, Jan 26, 2012 at 04:02:54PM +0200, Jan-Carel Brand wrote: On Wed, 2012-01-25 at 18:56 +0100, Souheil CHELFOUH wrote: A quick note : This quite an advanced vocabulary, why not make another package with a dependency on zope.schema ? I don't quite see the point to have that in the core. Ok, Charlie also expressed his reservations. I'll put it in a different package then. I'm not too sure what to name it though. For example, under what namespace? zope or z3c? My gut feeling says z3c. OTOH I don't think a single class warrants a whole separate PyPI package. Especially if it's going to be used with z3c.form (hint, hint ;) or that collective.forgotthenamealready you mentioned upthread. I'm guessing zope.vocabulary, or rather zope.treevocabulary? (Or z3c.treevocabulary) But I'd rather see this in zope.schema. Incidentally, since this would be a new feature, it would need a minor version bump in setup.py (4.0.2dev - 4.1dev). Furthermore, for the dict class in use in the vocabulary, you could add a factory class that can be overriden easily. That would allow people with OrderDict capabilities to use them without having to re-sort later on. Could you please elaborate on what you mean? I think he meant something like class TreeVocabulary(object): # you can subclass and use OrderedDictionary instead dict_factory = dict This would mean that you can't subclass dict and would instead have to delegate __getitem__, __iter__, __len__, keys, values, items and all the rest by hand. I'm actually feeling a bit guilty about raising this question. As I said, I don't have a use-case for ordered TreeVocabularies myself. (Or unordered ones either, for that matter ;) The only reason for hashing this out now is to avoid painful API changes if we ever decide that we need those. Feel free to cry YAGNI at any time. It's already hard enough to contribute to zope3-or-is-it-bluebream-or-is-it-ztk-aaaugh as it is, without us adding extra barriers in place. If I create a factory class to create TreeVocabulary instances, how will overriding that factory (without creating a separate SortableTreeVocabulary) allow people to use OrderedDict? Incidentally, I came upon this: http://pypi.python.org/pypi/ordereddict which provides the OrderedDict to Python 2.4 to 2.7 Oh, neat. I've used http://pypi.python.org/pypi/odict in the past, but it doesn't have the blessing of stdlib'ness. I think it might make sense to just subclass OrderedDict and implement an ordered tree from the start. By the way, good work on that, it's something that is often needed in advanced forms. I'll make sure to try it. Thank you for the effort. Thanks! Much appreciated. Thanks! Marius Gedminas -- http://pov.lt/ -- Zope 3/BlueBream consulting and development ___ 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 ) ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hi Souheil On Wed, 2012-01-25 at 18:56 +0100, Souheil CHELFOUH wrote: A quick note : This quite an advanced vocabulary, why not make another package with a dependency on zope.schema ? I don't quite see the point to have that in the core. Ok, Charlie also expressed his reservations. I'll put it in a different package then. I'm not too sure what to name it though. For example, under what namespace? zope or z3c? I'm guessing zope.vocabulary, or rather zope.treevocabulary? Furthermore, for the dict class in use in the vocabulary, you could add a factory class that can be overriden easily. That would allow people with OrderDict capabilities to use them without having to re-sort later on. Could you please elaborate on what you mean? If I create a factory class to create TreeVocabulary instances, how will overriding that factory (without creating a separate SortableTreeVocabulary) allow people to use OrderedDict? Incidentally, I came upon this: http://pypi.python.org/pypi/ordereddict which provides the OrderedDict to Python 2.4 to 2.7 I think it might make sense to just subclass OrderedDict and implement an ordered tree from the start. By the way, good work on that, it's something that is often needed in advanced forms. I'll make sure to try it. Thank you for the effort. Thanks! Much appreciated. J-C - Souheil 2012/1/25 Marius Gedminas mar...@gedmin.as: 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 - 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 ) ___ 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 ) ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hiya, Am 26.01.2012, 15:02 Uhr, schrieb Jan-Carel Brand li...@opkode.com: Ok, Charlie also expressed his reservations. I'll put it in a different package then. Hang on a minute! While I'm not 100 % convinced of the need in the core I think a separate package just for TreeVocabulary would be splitting hairs. If z3c.form can use it then I think that is justification enough. I'm not too sure what to name it though. For example, under what namespace? zope or z3c? I'm guessing zope.vocabulary, or rather zope.treevocabulary? Furthermore, for the dict class in use in the vocabulary, you could add a factory class that can be overriden easily. That would allow people with OrderDict capabilities to use them without having to re-sort later on. Could you please elaborate on what you mean? If I create a factory class to create TreeVocabulary instances, how will overriding that factory (without creating a separate SortableTreeVocabulary) allow people to use OrderedDict? Incidentally, I came upon this: http://pypi.python.org/pypi/ordereddict which provides the OrderedDict to Python 2.4 to 2.7 I think it might make sense to just subclass OrderedDict and implement an ordered tree from the start. I agree. Despite my previous remark about class methods, I don't think we need to worry much about Python 2.4 and 2.5 and ordered dictionaries are just so damn useful that they've been added to the standard library. Back to bike-shedding. As I was intrigued by the whole thing I've spent some time looking at the code. I'm not too happy on the use of nested functions as I find they obscure code, particularly when used recursively. I think that createTree and recurse should be written as separately testable generators. I also don't see a need for createTerm in this particular class and the subsequent awkward call from createTree. As it stands it is copy paste both in method and test. If you must have it with the same implementation createTree = SimpleVocabulary.createTree does the job just fine but I don't see the advantage of cls.createTerm(*args) over SimpleTerm(*args) As I said this is bike-shedding but I think our source code should be written with a view to being read and probably copied verbatim. With that in mind I prefer readability and testability over integration. In the end it tends to make things easier to use. The exceptions where refactoring to produce slightly uglier code but with significant performance hopefully prove the rule. Charlie -- Charlie Clark Managing Director Clark Consulting Research German Office Kronenstr. 27a Düsseldorf D- 40217 Tel: +49-211-600-3657 Mobile: +49-178-782-6226 ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On Thu, Jan 26, 2012 at 04:02:54PM +0200, Jan-Carel Brand wrote: On Wed, 2012-01-25 at 18:56 +0100, Souheil CHELFOUH wrote: A quick note : This quite an advanced vocabulary, why not make another package with a dependency on zope.schema ? I don't quite see the point to have that in the core. Ok, Charlie also expressed his reservations. I'll put it in a different package then. I'm not too sure what to name it though. For example, under what namespace? zope or z3c? My gut feeling says z3c. OTOH I don't think a single class warrants a whole separate PyPI package. Especially if it's going to be used with z3c.form (hint, hint ;) or that collective.forgotthenamealready you mentioned upthread. I'm guessing zope.vocabulary, or rather zope.treevocabulary? (Or z3c.treevocabulary) But I'd rather see this in zope.schema. Incidentally, since this would be a new feature, it would need a minor version bump in setup.py (4.0.2dev - 4.1dev). Furthermore, for the dict class in use in the vocabulary, you could add a factory class that can be overriden easily. That would allow people with OrderDict capabilities to use them without having to re-sort later on. Could you please elaborate on what you mean? I think he meant something like class TreeVocabulary(object): # you can subclass and use OrderedDictionary instead dict_factory = dict This would mean that you can't subclass dict and would instead have to delegate __getitem__, __iter__, __len__, keys, values, items and all the rest by hand. I'm actually feeling a bit guilty about raising this question. As I said, I don't have a use-case for ordered TreeVocabularies myself. (Or unordered ones either, for that matter ;) The only reason for hashing this out now is to avoid painful API changes if we ever decide that we need those. Feel free to cry YAGNI at any time. It's already hard enough to contribute to zope3-or-is-it-bluebream-or-is-it-ztk-aaaugh as it is, without us adding extra barriers in place. If I create a factory class to create TreeVocabulary instances, how will overriding that factory (without creating a separate SortableTreeVocabulary) allow people to use OrderedDict? Incidentally, I came upon this: http://pypi.python.org/pypi/ordereddict which provides the OrderedDict to Python 2.4 to 2.7 Oh, neat. I've used http://pypi.python.org/pypi/odict in the past, but it doesn't have the blessing of stdlib'ness. I think it might make sense to just subclass OrderedDict and implement an ordered tree from the start. By the way, good work on that, it's something that is often needed in advanced forms. I'll make sure to try it. Thank you for the effort. Thanks! Much appreciated. Thanks! Marius Gedminas -- http://pov.lt/ -- Zope 3/BlueBream consulting and development 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hiya, Am 24.01.2012, 18:48 Uhr, schrieb Jan-Carel Brand li...@opkode.com: 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. No need to add doctests. It was more a comment on the docstring of one method in comparison with the others. 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. er, just because the existing documentation is pants doesn't mean it can't be improved upon! ;-) I'm still not sure about having TreeVocabulary in zope.schema if it is only going to be used with, shudder, Archetypes. On the one hand schema are theoretically dissociated from any form library and zope.form is already incomplete, on the other we try and avoid application-specific requirements in the libraries. All the more important to expand the documentation so that other libraries can benefit from the plumbing. Charlie -- Charlie Clark Managing Director Clark Consulting Research German Office Kronenstr. 27a Düsseldorf D- 40217 Tel: +49-211-600-3657 Mobile: +49-178-782-6226 ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hi Charlie On Wed, 2012-01-25 at 10:37 +0100, Charlie Clark wrote: Hiya, Am 24.01.2012, 18:48 Uhr, schrieb Jan-Carel Brand li...@opkode.com: 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. No need to add doctests. It was more a comment on the docstring of one method in comparison with the others. 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. er, just because the existing documentation is pants doesn't mean it can't be improved upon! ;-) I'm still not sure about having TreeVocabulary in zope.schema if it is only going to be used with, shudder, Archetypes. It's *not* for use with Archetypes. :) That's what for example Products.ATVocabularyManager is for. I just mentioned that this is a fairly common use-case in Plone, but up to now only with Archetypes, because a zope3-component type TreeVocabulary didn't exist yet. That's why I wrote this one. On the one hand schema are theoretically dissociated from any form library and zope.form is already incomplete, on the other we try and avoid application-specific requirements in the libraries. Sure. Like SimpleVocabulary, the Treevocabulary is not dependent on any form library. In my case, I use it with z3c.form, but it could also be used with for example zope.formlib or any other form library that couples with zope.schema. All the more important to expand the documentation so that other libraries can benefit from the plumbing. I'll see what I can do. JC ___ 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 )
Re: [Zope-dev] 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 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
A quick note : This quite an advanced vocabulary, why not make another package with a dependency on zope.schema ? I don't quite see the point to have that in the core. Furthermore, for the dict class in use in the vocabulary, you could add a factory class that can be overriden easily. That would allow people with OrderDict capabilities to use them without having to re-sort later on. By the way, good work on that, it's something that is often needed in advanced forms. I'll make sure to try it. Thank you for the effort. - Souheil 2012/1/25 Marius Gedminas mar...@gedmin.as: 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 - 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 ) ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On Fri, 2012-01-20 at 13:50 +0200, Jan-Carel Brand wrote: Hi all I've been working on porting the Dynatree (a dynamic tree-like) widget to z3c.form: https://github.com/collective/collective.dynatree My (temporary) fork is here: https://github.com/syslabcom/collective.dynatree And for this I needed a hierarchical tree-like vocabulary. So I've created a TreeVocabulary in zope/schema/vocabulary.py, based upon the existing SimpleVocabulary. Instead of fromValues or fromItems, it has fromDict, to construct the vocab from a dict. And the internal representation, self._terms, is a dictionary. My branch is here: http://svn.zope.org/zope.schema/branches/jcbrand-treevocabulary/ The only changes are the new TreeVocabulary in zope/schema/vocabulary.py and the tests for it in zope/schema/tests/test_vocabulary.py Can someone please take a look and give some feedback? 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. Should I rather create a ticket on launchpad? Or otherwise, should I just go ahead and merge the changes to trunk? Thanks JC ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hiya, Am 24.01.2012, 12:35 Uhr, schrieb Jan-Carel Brand li...@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. At least not all of the methods have doc strings and one of them seems to have a doctest. It would be nice to expand the README here. 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. 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 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. At the moment I'm still not quite sure whether this is required in zope.schema. Do widgets only exist for z3c.form? Charlie -- Charlie Clark Managing Director Clark Consulting Research German Office Kronenstr. 27a Düsseldorf D- 40217 Tel: +49-211-600-3657 Mobile: +49-178-782-6226 ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
Incidentally, please do not hijack existing threads when you start a new
topic, ok?
On Tue, Jan 24, 2012 at 01:35:49PM +0200, Jan-Carel Brand wrote:
On Fri, 2012-01-20 at 13:50 +0200, Jan-Carel Brand wrote:
Hi all
I've been working on porting the Dynatree (a dynamic tree-like) widget
to z3c.form:
https://github.com/collective/collective.dynatree
My (temporary) fork is here:
https://github.com/syslabcom/collective.dynatree
And for this I needed a hierarchical tree-like vocabulary.
So I've created a TreeVocabulary in zope/schema/vocabulary.py, based
upon the existing SimpleVocabulary.
Instead of fromValues or fromItems, it has fromDict, to construct the
vocab from a dict. And the internal representation, self._terms, is a
dictionary.
My branch is here:
http://svn.zope.org/zope.schema/branches/jcbrand-treevocabulary/
The only changes are the new TreeVocabulary in zope/schema/vocabulary.py
and the tests for it in zope/schema/tests/test_vocabulary.py
Can someone please take a look and give some feedback?
vocabulary.py, line 146:
Initialize the vocabulary given a dict of terms.
Please clarify what a 'dict of terms' means. AFAIC the *keys* of your
dict are terms (instances of SimpleTerm or whatever), and the values are
dicts representing the children.
vocabulary.py, line 150:
gne or more interfaces may also be provided so that alternate
s/gne/One/
test_vocabulary.py, line 223:
self.assertTrue(dict, type(v._terms))
s/assertTrue/assertEqual/
test_vocabulary.py, line 249:
len returns the number of all nodes in die dict
s/die/the/
lines 255-257:
self.assertTrue('Regions' in self.tree_vocab_2 and \
'Austria' in self.tree_vocab_2 and \
'Bavaria' in self.tree_vocab_2)
The backslashes at the end of each line are not necessary. Same on
lines 262-264.
test_vocabulary, lines 192-194: you create list_vocab and items_vocab
and then never use them.
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 think TreeVocabulary should have a corresponding interface
ITreeVocabulary.
The new getTermPath() method is currently undocumented.
What's the use case for a tree vocabulary? A widget that displays the
tree structure explicitly? 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'm unhappy about len(tree_vocab) !+ len(list(tree_vocab)).
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.
Should I rather create a ticket on launchpad? Or otherwise, should I
just go ahead and merge the changes to trunk?
Marius Gedminas
--
http://pov.lt/ -- Zope 3/BlueBream consulting and development
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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On 01/24/2012 03:06 PM, Charlie Clark wrote: Off the top of my head I don't know which Python versions support classmethods. Classmethods are fine in python2.4 Cheers -- Georges Racinet, http://www.racinet.fr, http://anybox.fr Zope/CPS OpenERP expertise, assistance development GPG: 0x4862FFF7 identi.ca twitter: gracinet signature.asc Description: OpenPGP 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
On Tue, 2012-01-24 at 16:07 +0200, Marius Gedminas wrote:
Incidentally, please do not hijack existing threads when you start a new
topic, ok?
Yes, that was an honest mistake, no ill intentions. Won't happen again.
On Tue, Jan 24, 2012 at 01:35:49PM +0200, Jan-Carel Brand wrote:
On Fri, 2012-01-20 at 13:50 +0200, Jan-Carel Brand wrote:
Hi all
I've been working on porting the Dynatree (a dynamic tree-like) widget
to z3c.form:
https://github.com/collective/collective.dynatree
My (temporary) fork is here:
https://github.com/syslabcom/collective.dynatree
And for this I needed a hierarchical tree-like vocabulary.
So I've created a TreeVocabulary in zope/schema/vocabulary.py, based
upon the existing SimpleVocabulary.
Instead of fromValues or fromItems, it has fromDict, to construct the
vocab from a dict. And the internal representation, self._terms, is a
dictionary.
My branch is here:
http://svn.zope.org/zope.schema/branches/jcbrand-treevocabulary/
The only changes are the new TreeVocabulary in zope/schema/vocabulary.py
and the tests for it in zope/schema/tests/test_vocabulary.py
Can someone please take a look and give some feedback?
vocabulary.py, line 146:
Initialize the vocabulary given a dict of terms.
Please clarify what a 'dict of terms' means. AFAIC the *keys* of your
dict are terms (instances of SimpleTerm or whatever), and the values are
dicts representing the children.
Ok, I've clarified that a bit more.
vocabulary.py, line 150:
gne or more interfaces may also be provided so that alternate
s/gne/One/
Fixed
test_vocabulary.py, line 223:
self.assertTrue(dict, type(v._terms))
s/assertTrue/assertEqual/
Fixed.
test_vocabulary.py, line 249:
len returns the number of all nodes in die dict
s/die/the/
Fixed.
lines 255-257:
self.assertTrue('Regions' in self.tree_vocab_2 and \
'Austria' in self.tree_vocab_2 and \
'Bavaria' in self.tree_vocab_2)
The backslashes at the end of each line are not necessary. Same on
lines 262-264.
Removed.
test_vocabulary, lines 192-194: you create list_vocab and items_vocab
and then never use them.
Removed.
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.
I think TreeVocabulary should have a corresponding interface
ITreeVocabulary.
I agree, done.
The new getTermPath() method is currently undocumented.
I added documentation.
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.
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.
I'm unhappy about len(tree_vocab) !+ len(list(tree_vocab)).
You're right. I fixed that.
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?
Thanks Marius, and Charlie, for taking the time to check the code. I
appreciate it!
JC
___
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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
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 li...@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 ___ 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
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 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 )
Re: [Zope-dev] TreeVocabulary in zope.schema.vocabulary
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 ___ 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 )
[Zope-dev] TreeVocabulary in zope.schema.vocabulary
Hi all I've been working on porting the Dynatree (a dynamic tree-like) widget to z3c.form: https://github.com/collective/collective.dynatree My (temporary) fork is here: https://github.com/syslabcom/collective.dynatree And for this I needed a hierarchical tree-like vocabulary. So I've created a TreeVocabulary in zope/schema/vocabulary.py, based upon the existing SimpleVocabulary. Instead of fromValues or fromItems, it has fromDict, to construct the vocab from a dict. And the internal representation, self._terms, is a dictionary. My branch is here: http://svn.zope.org/zope.schema/branches/jcbrand-treevocabulary/ The only changes are the new TreeVocabulary in zope/schema/vocabulary.py and the tests for it in zope/schema/tests/test_vocabulary.py Can someone please take a look and give some feedback? Thanks J-C ___ 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 )
