-
Notifications
You must be signed in to change notification settings - Fork 40
[Guided Setup] Optimize default tree creation #7661
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
Conversation
|
Fixed the delete empty rank issue by adjusting the code in the new tree creation code to make sure parent ranks are correctly set. |
emenslin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Make sure the trees were pre-loaded correctly.
- Make sure the tree was created correctly. An easy way to check is to compare it with the same tree created before the speed ups.
Testing instructions from the issue-7593 PR to check:
- Configure ranks for the geography and taxon trees.
- After the setup, make sure the default tree creation process started for the trees you selected to pre-load. (You will see notifications)
- If you chose a paleo/geo database then a chronostrat tree should also be imported automatically.
- If you chose to -not- pre-load the taxon tree, the taxon tree that was created should not have a root node.
- After the setup, check all the new trees, make sure the ranks respect your configuration.
- Creating a new discipline with the configuration tool should create an empty taxon tree.
- An empty taxon tree should have an import button
- Make sure you can import a taxon tree into the existing empty tree
- Make sure you can still create a new populated tree
Pre-loading geography trees and adding existing taxon through the taxon tree is much faster (~1-3 minutes depending on the tree) so that part looks good!
There's no option to pre-load taxon trees but it is included in the testing instructions so what is the plan for that? If it's not included in this PR that's fine but the testing instructions need to be updated. Also by default taxon tree has Subspecies checked as "in full name" but not included, was that intentional?
Storage tree separator default should be set to , but is currently set to . Also nothing is marked as "In full name" not sure if it's needed or not so I wanted to point it out in case.

Nothing is checked as "in full name" for Geography either.

All tree nodes are added under the root rank and duplicated

Taxon tree does not respect any customizations made on the setup page. During setup I included some ranks that were not included by default and when I opened the taxon tree after setup they weren't there.
I tested the setup twice and one of the times when I pressed the create button after set up it did the loading thing and then sent me back to the last setup page. Since it didn't happen both times I'm not sure if it's important or not but I wanted to make note of it.
|
Got a fix for the taxon rank issue in the batched generated tree. The generated trees should appear correct in the tree viewer now. Looks like this PR just adds the Geography preloads in the in the guided setup. The taxon trees can be generated/imported through the tree viewer. |
kwhuber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Test with the setup page
-
Test in the taxon tree viewer
- Add a new taxon tree and choose to import a populated one.
- Make sure the tree was created correctly. An easy way to check is to compare it with the same tree created before the speed ups.
- The speed performance of this is much improved!
|
Some of my changes weren't pushed initially leading to this PR being more broken than it should be. Closed in favor of #7669 (All additional fixes were ported over too). |




Fixes #7641
Speeds up default tree creation significantly.
Checklist
self-explanatory (or properly documented)
Testing instructions
Testing instructions from the issue-7593 PR to check: