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

fix get_distance() for topology_only=True #742

Open
wants to merge 1 commit into
base: ete3
Choose a base branch
from

Conversation

lenacoll
Copy link

@lenacoll lenacoll commented Feb 8, 2024

In tree.get_distance(target, target2, topology_only=True), the number of nodes between target and target2 was not calculated correctly.

In the while loop starting in line 1024, we add +1 for the parent of current This means that when reaching the children of ancestor, we count ancestor twice. The only exception to this is if one of the two nodes, say target is the ancestor -- but in this case we still do count ancestor when going through the while loop for target2, even though ancestor is target and should therefore not be counted.

So in both cases, we need to subtract one from the count of nodes between target and target2. In the previous version this has been done with the if condition in line 1026, which simply skips counting the parent of target, which does not work if target=ancestor.

I therefore deleted this if condition and instead added an if after the while loop in line 1030 to subtract one from the distance computed in the while loop.
We need to check that target != target2, as in this case the ancestor is the node that's given, so we would not go into the while loop and therefore don't need to subtract one from the distance computed.

This addresses issue #740.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant