[ZCM] [ZC] 2040/ 5 Comment "Patch to make PathIndex sortable"

"Collector: Zope Bugs, Features, and Patches ..." <[email protected]> Wed, 04 Jul 2007 15:08:15 -0400
Newsgroups gmane.comp.web.zope.devel.collector-monitor
Message-ID <[email protected]>
Issue #2040 Update (Comment) "Patch to make PathIndex sortable"
 Status Pending, Catalog/feature+solution medium
To followup, visit:
  http://www.zope.org/Collectors/Zope/2040

==============================================================
= Comment - Entry #5 by mcdonc on Jul 4, 2007 3:08 pm

FWIW, I'm not sure about the "index" implementation but this patch seems to work for me.  A pathindex sort_on will return brains sorted in a depth-first order (which is useful for building nav trees).  Thank you much Nick! ;-)
________________________________________
= Comment - Entry #4 by suvit on Mar 16, 2006 9:43 am

2. Because:

1. this code works slow.
Catalog.sortResults only needed (key, intset) from 'items' method, but simple int value is already a intset.
introspection, union, difference with int values works fine.
so code change into
    def items(self):
        return self._index.items()
2. but PathIndex`s self._index is not the same as Unindex._index.
the structure of the PathIndex._index BTree is more complex. So it doesn`t match to us.

3. The Catalog.uids BTree is that is needed. but ugly Catalog._getSortIndex method get sort_index without acquisition( this easy to patch), and it is impossible to get the 'Catalog' instance from 'Index' instance. 
________________________________________
= Comment - Entry #3 by nickbower on Mar 14, 2006 9:43 pm

Good spot.  Couple of points.

1) Should we include "def items" in ISortIndex?  Interfaces allow people (like me) who don't know every single code inter-dependency to implement additional behaviour.  As I wasn't aware of Catalog.sortResults, the need for "items" was undetectable.

Actually, "items" is not in *any* interface...

2) Why not just the following?

    def items(self):
        items = []
        for k,v in self._index.items():
            if isinstance(v, int):
                v = IISet((v,))
            items.append((k, v))
        return items



________________________________________
= Comment - Entry #2 by suvit on Mar 9, 2006 7:01 am

do not forget about the 'items' method, which is needed in some cases by Catalog.sortResults method. SortIndex interface missing 'items' method.

may be 'items' method implementation:

def items(self):
   aq_parent(self).uids.items()

but if PathIndex instance gotten without acquisition, this method will work wrong.

________________________________________
= Request - Entry #1 by nickbower on Mar 6, 2006 8:55 pm


Uploaded:  "PathIndex.patch"
 - http://www.zope.org/Collectors/Zope/2040/PathIndex.patch/view
PathIndex (in PluggableIndexes) is not sortable because it doesn't inherit from the PluggableIndexes/common/UnIndex.py (like FieldIndex etc do).  I can see no obvious reason why it couldn't be sortable hoewver as it still implements inverse lookups using self._unindex which is an IOBTree.

I have produced a patch and attach it here.  Unit tests ran fine and when using it, PathIndex won't barf when you supply the "sort_on" attribute to a catalog search on on this index.

Could this be considered for inclusion into the next zope release?

==============================================================