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

Add feature in populate method to sample from common tree distributions #714

Merged
merged 16 commits into from
Nov 20, 2023

Conversation

harryrichman
Copy link

@harryrichman harryrichman commented Oct 13, 2023

The populate method is modified to have an additional distribution argument, which can select from three possible options:

  • distribution="fast": populates a subtree using the previous algorithm.
  • distribution="yule": populate a subtree according to the Yule distribution.
  • distribution="uniform" or distribution="pda": populate a subtree according to the uniform or PDA (proportional to distinguishable arrangements) distribution.

These distributions are defined in Semple and Steel, Phylogenetics, Chapter 2.5.

This pull request also adds the following features to the populate method:

  • The newly-generated subtree is ladderized. This behavior can be controlled with the ladderize boolean keyword argument, with default value True.
  • There is an optional seed keyword argument, which can be used to control the random behavior of subtree generation if desired.

Closes #691

@harryrichman
Copy link
Author

@jordibc Just wanted to check, is this pull request under consideration currently? I saw you mention in my other pull request that ete3 is not longer being actively developed, but this could be considered a bugfix since the current documentation of populate may lead users (including some colleagues of mine) to assume it produces a random topology from the uniform distribution.

Thanks!

@jordibc
Copy link
Contributor

jordibc commented Nov 17, 2023

Hi @harryrichman , yes, we were considering this and didn't decide yet -- mainly because of working on new features for ete4 and preparing a workshop.

Your contribution looks very good (the idea, the implementation, the description with references and the explanation in the docstring). We almost surely would love to add it and do it soon. We postponed it because we prefer not to add functionality to ete3 and concentrate on ete4 and we normally ask for tests with the new code. We would probably leave ladderize out from the options too, but that's very minor. In any case we plan to add this to ete4 and exceptionally to ete3 too.

Many thanks for it and for making it so clean!

@jordibc jordibc merged commit c7a0685 into etetoolkit:ete3 Nov 20, 2023
jordibc added a commit that referenced this pull request Nov 20, 2023
…nd support functions.

This version incorporates ideas from #714

We can now select model='yule' or 'uniform'.

And we have more flexibility with how the distances and supports are
added: we can add either random distances and/or random supports,
drawn from whatever distribution we choose, by passing the appropriate
functions as arguments (instead of the tuples of (min,max) that we had
for uniform distributions of both before).

Also, a significant clean up of the code, and updated the tests.
@jordibc
Copy link
Contributor

jordibc commented Nov 20, 2023

@harryrichman You may want to know that since commit 1fff062 we also added this functionality to ete4, with some changes to the api that we hope makes it cleaner and more useful. Thanks again!

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.

2 participants