Preserve branch length in Phylo.TreeMixin.prune if keep_root_length=True

[michaelfm1211]

• I hereby agree to dual licence this and any previous contributions under both
  the Biopython License Agreement AND the BSD 3-Clause License.

• I have read the CONTRIBUTING.rst file, have run pre-commit
  locally, and understand that continuous integration checks will be used to
  confirm the Biopython unit tests and style checks pass with these changes.

• I have added my name to the alphabetical contributors listings in the files
  NEWS.rst and CONTRIB.rst as part of this pull request, am listed
  already, or do not wish to be listed. (This acknowledgement is optional.)

Closes #4460

Currently, when running .prune() on a taxon in a bifurcation from the root, the root is collapsed so the other node has its length set to zero and becomes the new root. In the linked issue, setting the length to zero was causing problems.

The length-to-zero behavior is explicitly commented as being intentional, however, the cookbook documentation for .prune() admits "[the branch length] might no longer be a meaningful value".

This PR adds a new keep_root_length keyword argument to the .prune() method that, when set to True, adds the parent and remaining node lengths together like any other bifurcation. To keep compatibility and because this behavior intentional, keep_root_length's default value is False so this PR only does anything if the end-user opts-in.

[michaelfm1211]

Ok, the tests are now passing. Before some doctests failed because the .prune() tests required another example tree to work with in the example PhyloXML file, but nothing to do with the actual code.

[michaelfm1211]

@peterjc @mdehoon Any thoughts on this? This PR doesn't change any default behavior (it just allows the user to opt-out of one feature), so it should be a pretty small change.

[mdehoon]

@michaelfm1211 I don't have a strong opinion about this PR, but can you explain when the new argument would be used in practice?

[michaelfm1211]

@mdehoon It would allow users to prune a tree and have the branch lengths of the resulting tree add up to what they did before the pruning.

For example, consider this tree (a simplified version of the example in the linked issue #4460):

Tree(rooted=False, weight=1.0)
    Clade(branch_length=0.0)
        Clade(branch_length=27.0, name='internal_3')
        Clade(branch_length=0.0, name='diploid')

After pruning the node with name='diploid' without this new argument, you get the resulting tree:

Tree(rooted=False, weight=1.0)
    Clade(name='internal_3')

In the original, the branch length of the name='internal_3' node was 27, but after pruning it is 0. This could be problematic depending on what branch length means to the user and was problematic for the user who submitted the linked issue.

If the same pruning was done with keep_root_length=True, then the branch length would remain 27.

[mdehoon]

@michaelfm1211 Thank you. Do you think that the default behavior is a bug? (I am not an expert in this area.)

[peterjc]

I would defer to @etal - I have worked with trees but not recently enough to have a quick judgement.

[michaelfm1211]

@mdehoon The current code explicitly states in a comment that the current behavior is not a bug, so I think it would be best for compatibility for the default behavior to not change.