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

Change Tree.mrca signature to take > 2 args #1340

Closed
jeromekelleher opened this issue May 5, 2021 · 2 comments · Fixed by #2121
Closed

Change Tree.mrca signature to take > 2 args #1340

jeromekelleher opened this issue May 5, 2021 · 2 comments · Fixed by #2121
Labels
enhancement New feature or request Python API Issue is about the Python API
Milestone

Comments

@jeromekelleher
Copy link
Member

jeromekelleher commented May 5, 2021

Following the discussion in #1339, we should change the signature of Tree.mrca to accept two or more nodes. The initial implementation can be something like

def mrca(self, *args):
   if len(args) < 2:
        raise ValueError("Must supply at least two arguments")
   return functools.reduce(self._ll_tree.mrca, args)

We're not going to break anyone's code by doing this, and it's an obvious generalisation. We can make a more performant implementation in the future if we like, but this'll do fine for now.

@jeromekelleher jeromekelleher added enhancement New feature or request Python API Issue is about the Python API labels May 5, 2021
@benjeffery benjeffery added this to the Python 0.3.7 milestone May 6, 2021
@hyanwong
Copy link
Member

This would be useful to find the MRCA of all samples in the tree (which could be different from the root if there are unary nodes above the grand MRCA). Would it be much slower than looking at the root and descending until we find a non-unary node, though?

@jeromekelleher
Copy link
Member Author

I think we should make this a different function (and issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Python API Issue is about the Python API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants