Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved Descendant Tree and Ancestor Tree new features #1873

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dave-khuon
Copy link
Contributor

new Descendtree.py was based on recently outdated code. Just re-applied changes to the new version.

new Descendtree.py was based on recently outdated code.  Just re-applied changes to the new version.
@dave-khuon
Copy link
Contributor Author

This PR should have been named "Improved Descend Tree". It is my mistake.

Please let me know when I can create another PR for a similar Tree (Ancestor Tree) that depends on the new gen/plug/report/utils.py pushed by this PR #1873.

@dave-khuon
Copy link
Contributor Author

To help explain the nature of the PR, let me share the test results:
. descendant tree without fill color option
. descendant tree with fill color option
. family descendant tree without fill color option
. family descendant tree with fill color option

test_descend_chart_withcolor.pdf
Test_descend_chart_withoutcolor.pdf
Test_family_descend_chart_withcolor.pdf
Test_family_descend_chart_withoutcolor.pdf

@dave-khuon dave-khuon changed the title Submit with new fixes Improved Descendant Tree - resubmit with new fixes Jan 25, 2025
@Nick-Hall
Copy link
Member

You should implement colour options in the same way as the graph reports. Perhaps it is time to create a standard option for this?

@dave-khuon
Copy link
Contributor Author

In the beginning, I was thinking of doing that too. I wanted to implement a common function in report/utils.py. However, due to the way this Tree Chart uses box styles, I was not able to dynamically modify the box color for each person's gender. To make it work, I end up creating a set of 2 boxes for each gender (normal and bolded) at the outset. There are other graph reports based on graphviz that seem to be able do that.

Their names are specific to each report. Maybe there are other ways. I am open to suggestions, to learn more about how the boxes work.

PS: I may also handle the creation and switching of the personbox names in a function in utils.py. I feel that in doing so, I am putting a distance between the report and its person boxes. Ideally, I would like to get some help if we can dynamically modify the box color as we navigate along the tree.

@Nick-Hall Nick-Hall added this to the v6.1 milestone Jan 27, 2025
Hi Nick,
I refactored the creation and invocation of styles by gender and family boxes to the utils class.  I am still using style sheets to control the box color.  Is it possible to consider this as a stop-gap solution, for the time being?

I am also looking at using graph reports such as family-line and hour-glass.  It would be a major rewrite, in my estimation.  I like their extra capability, and I am thinking of creating graph-based reports for ancestors and descendants that allow "scale to fit page width only", which is important for me, considering the huge number of nodes for my family.  But this is going to take a lot of research and coding.  I am not confident that I can do it on my own.
Fix list declaration
@dave-khuon
Copy link
Contributor Author

Hi Nick,

Please review my note on "Re-submit after refactoring" about a stop-gap solution.

Change report/utils.py class to:
. use color scheme from config class
. modify the parameter list of generate_gender_color_styles and generate_family_color_style to include a base_draw_style name.  Change the descendtree.py accordingly.
@dave-khuon
Copy link
Contributor Author

Is the link "Try the new merge experience" supposed to be used by the author of the PR?
image

@bnapolitano
Copy link

New Test results: IngermanSmith_NoColor_family_descend_chart.pdf
IngermanSmith_Color_family_descend_chart.pdf
IngermanSmith_NoColor_descend_chart.pdf
IngermanSmith_Color_descend_chart.pdf

The colors are a nice touch, but comparing the files, the non-color PDFs show the marriage event but the color PDFs seem to duplicate one of the spouses rather than show the marriage event.

Even though this is a different report, it is also dependent on the same report/utils class, as the result of refactoring the function to handle the color of the boxes.  The code changes are similar between the 2 reports.  I am requesting that this feature be extended to this report as well.  It will make it easier for the reviewing and approval process of the PR easier than having to deal with two.
@dave-khuon
Copy link
Contributor Author

@dave-khuon dave-khuon changed the title Improved Descendant Tree - resubmit with new fixes Improved Descendant Tree and Ancestor Tree new features Jan 31, 2025
@Nick-Hall
Copy link
Member

Is the link "Try the new merge experience" supposed to be used by the author of the PR?

You can read about the public preview on the GitHub blog. It's mainly for people who merge pull requests, but the improved layout of checks may help developers.

Fixed by moving the set box color to the end of the method.  See new test results
@dave-khuon
Copy link
Contributor Author

dave-khuon commented Jan 31, 2025 via email

Add try / except to handle bad data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants