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

incorrect json -> Bio.Phylo conversion #714

Closed
rneher opened this issue Apr 12, 2021 · 0 comments · Fixed by #908
Closed

incorrect json -> Bio.Phylo conversion #714

rneher opened this issue Apr 12, 2021 · 0 comments · Fixed by #908
Assignees
Labels
bug Something isn't working

Comments

@rneher
Copy link
Member

rneher commented Apr 12, 2021

The utility function json_to_tree does not correctly calculates branch length. Instead of calculating the difference between divergence of parent and child node, it simply assigns divergence:

https://github.com/nextstrain/augur/blob/master/augur/utils.py#L589

@rneher rneher added the bug Something isn't working label Apr 12, 2021
@huddlej huddlej self-assigned this Apr 13, 2021
huddlej added a commit that referenced this issue Apr 27, 2022
Sets branch length to the difference in divergence between each node and
its parent instead of setting the cumulative branch length from the root
of the tree. Auspice JSONs store the cumulative branch length for easier
plotting in Auspice, but when we import the JSON into Python as a tree,
the branch length should not be cumulative. Since this cumulative branch
length is useful on its own and also required for calculating the actual
branch length (in the current recursive implementation of
`json_to_tree`, at least), this commit adds a `cumulative_branch_length`
attribute to each node of the tree produced by `json_to_tree` and passes
this value as a new optional argument to recursive calls.

We should really reimplement this function as a non-recursive function,
though. See the cartography script for annotating Auspice JSONs [1], for
an example of this kind of non-cursive implementation.

Fixes #714

[1] https://github.com/blab/cartography/blob/master/notebooks/scripts/annotate_tree.py
@huddlej huddlej moved this from New to In Review in Nextstrain planning (archived) Apr 27, 2022
Repository owner moved this from In Review to Done in Nextstrain planning (archived) Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants