Add feature in `populate` method to sample from common tree distributions

etetoolkit / ete

Python package for building, comparing, annotating, manipulating and visualising trees. It provides a comprehensive API and a collection of command line tools, including utilities to work with the NCBI taxonomy tree.

http://etetoolkit.org

GNU General Public License v3.0

768 stars 216 forks source link

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

Closed harryrichman closed 7 months ago

harryrichman commented 9 months ago

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 commented 7 months ago

@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 commented 7 months ago

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 commented 7 months ago

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