jeetsukumaran / DendroPy

A Python library for phylogenetic scripting, simulation, data processing and manipulation.
https://pypi.org/project/DendroPy/.
BSD 3-Clause "New" or "Revised" License
210 stars 61 forks source link

suppress_unifurcations=False when computing bipartitions after unrooting is broken #102

Closed thekswenson closed 5 years ago

thekswenson commented 6 years ago

The code tree2.is_rooted = False; tree2.update_bipartitions(suppress_unifurcations=False) no longer works for unrooting a tree.

For an example, loot at https://dendropy.org/primer/treemanips.html#setting-the-rooting-state. The output is now:

After `update_bipartitions()`:
/---------------------------------------------------------------------------- A
|                                                                              
+---------------------------------------------------------------------------- B
|                                                                              
|                        /--------------------------------------------------- C
\------------------------+                                                     
                         |                         /------------------------- D
                         \-------------------------+                           
                                                   \------------------------- E

After `update_bipartitions(suppress_unifurcations=False)`:
/---------------------------------------------------------------------------- A
|                                                                              
+---------------------------------------------------------------------------- B
|                                                                              
|                        /--------------------------------------------------- C
\------------------------+                                                     
                         |                         /------------------------- D
                         \-------------------------+                           
                                                   \------------------------- E

rather than:

After `update_bipartitions()`:
/---------------------------------------------------- A
|
+---------------------------------------------------- B
|
|                /----------------------------------- C
\----------------+
                 |                 /----------------- D
                 \-----------------+
                                   \----------------- E

After `update_bipartitions(suppress_unifurcations=False)`:
/---------------------------------------------------- A
+
|            /--------------------------------------- B
\------------+
             |            /-------------------------- C
             \------------+
                          |            /------------- D
                          \------------+
                                       \------------- E
jeetsukumaran commented 6 years ago

Thanks for pointing this out. Will look into this eventually. Feel free to update here to bump this as a reminder if too long passes ...

sjspielman commented 5 years ago

I'm seeing this behavior as well.. gentle ping? Thanks!

jeetsukumaran commented 5 years ago

Ah, apologies for the confusion.

A unifurcation is an internal node with an outdegree of one (i.e., only one child).

+---+---A

There are no unifurcations on the trees in the above examples. What you are seeing is a basal trifurcation. A basal trifurcation is required on unrooted tree with bipartition encoding because with a basal bifurcation, the two child edges will have the same hash value. In fact, the bifurcating root on an unrooted tree is artifactual --- it does not really exist, and is just a misleading side-effect of newick notation.

I am guessing that what you want avoided is the collapse of the basal bifurcation to a basal trifurcation? Apologies for the documentation being out-of-date and misleading about this. With the more recent versions of DendroPy, you need to pass in collapse_unrooted_basal_bifurcation=False. At least this API makes better sense!

tree2.update_bipartitions(collapse_unrooted_basal_bifurcation=False)

More information here: