From: Nick H. <nic...@ho...> - 2009-10-30 22:01:45
|
Benny, I have uploaded another patch. Removing the helper function and using the TYPE2GROUP lookup is a good idea to improve performance. I was using EventType instances rather than the numeric constants to refer to event types because I wanted to distinguish between different custom events. This was only going to be useful in the future (possibly?), so I don't have any problem with your change. Regarding the points you raised on bug tracker: "1/test this patch, works on my data, but I have almost only birth and death events here" I tested your patch and it worked fine. "2/explain me what group_size does. The code has: group_size = len(GROUP_DICT[event_group]) and then a check if that is 1 or not. I don't understand this. Also, in my changes, custom events are only present as eventtype.CUSTOM, so they become =1 now! Also, check on this size looks dangerous as the table GROUP_DICT at the top of the module can be changed without giving indication this changes the logic." The group_size is how many event types are in the event group and hence the section. Where there is only one event type in the group it looked silly listing the same event type in the first column of every row of the section. So I took it out and moved the description over. After looking at this again I have made 3 changes: a. I have put the date in the first column instead of description. When the description is long it was making each event take up 2 lines. Dates tend to fit into the first column with no problems. b. I have coded it so that the custom events section always displays the event type. c. I have removed the role where the was no event type displayed. It looked wrong. "3/the tab with sections is crowded. Can you investigate if it is possible to put at the top of the tab with sections a label with explenation for the user, so it is clear what checking these options does. It would be nice if this section could be split in two columns :-)" I have split the sections into two columns and added a label. Regards, Nick. |
From: Benny M. <ben...@gm...> - 2009-10-31 10:07:15
|
Everybody, I would like to give NIck commit access if nobody objects. His coding is of good quality, and he helps in the core parts too. I am sure Nick understands that changes in the core need some reflection before a commit, and that the devel list is the way to flesh things out. Nick, do you have a sourceforge login? I would need to know it to give access. Benny 2009/10/30 Nick Hall <nic...@ho...>: > Benny, > > I have uploaded another patch. > > Removing the helper function and using the TYPE2GROUP lookup is a good > idea to improve performance. > > I was using EventType instances rather than the numeric constants to > refer to event types because I wanted to distinguish between different > custom events. This was only going to be useful in the future > (possibly?), so I don't have any problem with your change. > > Regarding the points you raised on bug tracker: > > "1/test this patch, works on my data, but I have almost only birth and > death events here" > > I tested your patch and it worked fine. > > "2/explain me what group_size does. The code has: > group_size = len(GROUP_DICT[event_group]) > and then a check if that is 1 or not. I don't understand this. Also, in > my changes, custom events are only present as eventtype.CUSTOM, so they > become =1 now! Also, check on this size looks dangerous as the table > GROUP_DICT at the top of the module can be changed without giving > indication this changes the logic." > > The group_size is how many event types are in the event group and hence > the section. Where there is only one event type in the group it looked > silly listing the same event type in the first column of every row of > the section. So I took it out and moved the description over. After > looking at this again I have made 3 changes: > > a. I have put the date in the first column instead of description. When > the description is long it was making each event take up 2 lines. Dates > tend to fit into the first column with no problems. > > b. I have coded it so that the custom events section always displays the > event type. > > c. I have removed the role where the was no event type displayed. It > looked wrong. > > "3/the tab with sections is crowded. Can you investigate if it is > possible to put at the top of the tab with sections a label with > explenation for the user, so it is clear what checking these options > does. It would be nice if this section could be split in two columns :-)" > > I have split the sections into two columns and added a label. > > Regards, > > Nick. > > > ------------------------------------------------------------------------------ > Come build with us! The BlackBerry(R) Developer Conference in SF, CA > is the only developer event you need to attend this year. Jumpstart your > developing skills, take BlackBerry mobile applications to market and stay > ahead of the curve. Join us from November 9 - 12, 2009. Register now! > http://p.sf.net/sfu/devconference > _______________________________________________ > Gramps-devel mailing list > Gra...@li... > https://lists.sourceforge.net/lists/listinfo/gramps-devel > |
From: Brian M. <br...@gr...> - 2009-10-31 12:54:18
|
Benny, > I would like to give NIck commit access if nobody objects. > His coding > is of good quality, and he helps in the core parts too. > I am sure Nick understands that changes in the core need > some > reflection before a commit, and that the devel list is the > way to > flesh things out. I agree. ~Brian |