Menu
▾
▴
Re: [Gramps-devel] Bug and missing methods in proxies
|
From: Doug B. <dou...@gm...> - 2010-05-23 12:32:24
|
On Sun, May 23, 2010 at 8:21 AM, Gerald Britton <ger...@gm...> wrote: > On Sun, May 23, 2010 at 7:54 AM, Doug Blank <dou...@gm...> wrote: >> On Sun, May 23, 2010 at 7:38 AM, Gerald Britton >> <ger...@gm...> wrote: >>> On Sun, May 23, 2010 at 1:18 AM, Doug Blank <dou...@gm...> wrote: >>>> Gerald, >>>> >>>> I believe that I fixed a bug in the filter proxy code [1] and added >>>> three missing methods in the living proxy class [2]. Please let me >>>> know if those look like appropriate fixes. Thanks, >>>> >>>> -Doug >>>> >>>> PS - I found these by playing around with a way to visualize the proxies. >>>> >>>> [1] - http://gramps.svn.sourceforge.net/viewvc/gramps?view=rev&revision=15451 >>> >>> There is something wrong with this update. The original code is just: >>> >>> def iter_person_handles(self): >>> """ >>> Return an iterator over database handles, one handle for each Person in >>> the database. >>> """ >>> return self.plist >>> >>> which I believe to be correct. However the diff refers to 15450, >>> which I cannot see in SVN at all. From what I can see, the original >>> code (above) is correct, unless you can provide a fail scenario >> >> Gerald, >> >> Change #15451 refers to: >> >> http://gramps.svn.sourceforge.net/viewvc/gramps/trunk/src/gen/proxy/filter.py?r1=15451&r2=15450&pathrev=15451 >> >> which is about iter_people. It was: >> >> for handle in self.plist: >> yield handle, self.get_person_from_handle(handle) >> >> which does not match what gen.db does. > > Not, that's not the original. The original just returns self.plist, > which I think is correct behavior. I cannot see where the for-loop > was introduced. Looking at the annotated version (I'm in trunk): http://gramps.svn.sourceforge.net/viewvc/gramps/trunk/src/gen/proxy/filter.py?annotate=15451&pathrev=15451 it looks like you added that 13140. If plist is a list of handles, then iter_people should return people, and get_person_from_handle should do the correct thing. > The previous rev (13676), is just: > > def iter_person_handles(self): > > return self.plist > > I believe that this is correct as is and do not know where the code > came from that your rev is based on. Clearly the code your rev is > based on is incorrect, but it differs from the original and is itself > incorrect. > >> >>>> [2] - http://gramps.svn.sourceforge.net/viewvc/gramps?view=rev&revision=15453 >>> >>> This should not be needed. The method include_person provides the >>> test needed and the standard methods in ProxyBase call this method >>> before including data. >>> >>> Can you provide a fail scenario? >> >> These changes need to have the self.__restrict_person(person) applied >> in the iterators. Nothing does that in the ProxyBase, correct? > > I see. You want to do post processing of the data before returning it. > In that case, you need iter_people but not iter_person_handles nor > get_person_handles. Note that __restrict_person always returns a > Person object. Ok, thanks. -Doug >> >>> Also, this is considered bad form: >>> >>> handles = [] >>> 136 for handle in self.db.get_person_handles(): >>> 137 person = self.get_person_from_handle(handle) >>> 138 if person: >>> 139 handles.append(handle) >>> >>> You should always use a list generator instead: >>> >>> handles = list(handle for handle in self.db.iter_person_handles() >>> if self.get_person_from_handle(handle)) >> >> Yes, nicely written. >> >>> but, as I said above, I do not believe that this is needed at all. >> >> Let me know... I don't see the living proxy applied if I iterate. >> >> -Doug >> >>>> >>> >>> >>> >>> -- >>> Gerald Britton >>> >> > > > > -- > Gerald Britton > |