Enhance Ancestor and Descend Trees to use color based on gender, life status and marriage type#2370
Conversation
I have used the strategy below: Utils: 1. Borrow the Person Gender only once per report 2. Borrow the Family Relationship Type only one per report 3. Generate a set of generic Style Boxes with specific color for each combination of genders and life status. 4. Generate a set of generic Style Boxes with specific color for each type of Marriage. For both tree reports: 1. Call generate_ functions in utils to generate both sets of person style boxes and marriage style boxes. 2. When navigating through the family tree, apply the right color by picking the right Style Box, based on the person gender and life status, and on the marriage type. I hope that will keep the processing of the reports efficient. This is a new PR on a new branch based on master repo only. It supersedes both PR 1873 (partial), and PR 2300 (full features), to simplify review, and my lack of skills in doing githup! In order to preserve the results of reviews given by DSBank to the above 2 PRs, I am including below an extract of the list of his issues and recommendations: A) I couldn't find the 1st issue raised by DsBank pertaining to the inefficiency of invoking several big objects, just to get gramps constants, and setup to a "static set of colors for people and marriage". I have addressed that following my above strategy. B) DSBank: Yes, almost there — three small things to fix, then I can approve: 1. is_alive is computed but never used (both files) In descendtree.py:129 and ancestortree.py:108: is_alive = probably_alive(person, database) This result is thrown away immediately — the color selection ignores it. Either remove the line (since alive/dead distinction isn't implemented yet), or actually wire it up to choose between colors.male-alive vs colors.male-dead. The current state wastes a database call on every person box. 2. self.boldable is stored but never read (descendtree.py:121) self.boldable = boldable Nothing reads self.boldable anywhere. It's a dead attribute — delete it (and the boldable=0 parameter too, unless libtreebase calls PersonBox with that argument). 3. Remove the PR-referencing comments from utils.py # for this PR: _G_ALIVE is used regardless of the actual alive status of the person. # The same goes with _F_MARRIED for families. # The next PR will expand on these options. Comments that say "this PR" and "the next PR" rot immediately and confuse future readers. If you want to document the limitation, replace with something timeless like: # Currently uses the alive-color for all persons regardless of actual alive status. Everything else looks good: Gender color mapping bug (UNKNOWN/OTHER swap) is fixed ✓ bold_now state machine is correct ✓ Bold-direct-descendant regression is fixed ✓ Debug comment removed ✓ Help text capitalization fixed ✓ get_report_family_colors is now a plain list (not a dict with opaque key 0) ✓ Colors computed once per run (not at import time) ✓ Black formatting passes ✓ Fix those three items and I'll approve. Thanks again for your help. I have learned a lot in this exercise.
dave-khuon
changed the title
dave-khuon
changed the title
|
I made a few changes to the sample family marriage type to verify whether we got the intended effects: Smith, Martin - Jefferson, Elna: Civil Union It seems we got the right coloring based on person gender, and life status, and marriage type. Please check these reports: Sample Family_family_descend_chart.pdf I also close the old PR 1873, 2300 since they are being replace by this PR. |
|
In order to preserve the results of reviews given by DSBank to the 2 replaced 2 PRs (1873, 2300), I am including below an extract of the list of his issues and recommendations: A) I couldn't find the 1st issue raised by DsBank pertaining to the inefficiency of invoking several big objects, just to get gramps constants, and setup to a "static set of colors for people and marriage". I believe these issues have addressed. B) DSBank: is_alive = probably_alive(person, database) self.boldable is stored but never read (descendtree.py:121) Remove the PR-referencing comments from utils.py Currently uses the alive-color for all persons regardless of actual alive status. Everything else looks good: Thanks again for your help. I have learned a lot in this exercise. |
2 participants
This PR supersedes both PR 1873 (partial), and PR 2300 (full features), to simplify review, and overcome my mess with stacking branches. Both old PR# have been de-activated "with prejudice" (=closed).
This PR will make both the Ancestor and Descend Trees use appropriate color for person and marriage based on the gender, life status, and marriage type. The main logics are in the shared gen/plug/report/utils.py.