PoonLab / Kaphi

Kernel-embedded ABC-SMC for phylodynamic inference
GNU Affero General Public License v3.0
4 stars 2 forks source link

Implement a check for malformed Newick tree strings #70

Closed gtng92 closed 7 years ago

gtng92 commented 7 years ago

from closing of Issue #55

ArtPoon commented 7 years ago

To reproduce:

> require(Kaphi)
> tr <- "((A:1,B:1):1,C:1));"
> tree <- read.tree(text=tr)
> tree

Phylogenetic tree with 3 tips and 3 internal nodes.

Tip labels:
[1] "A" "B" "C"
Node labels:
[1] "" NA ""

Rooted; includes branch lengths.

# ape read.tree parses the malformed Newick without complaining!

> plot(tree)
Error in plot.phylo(tree) : 
  tree badly conformed; cannot plot. Check the edge matrix.
> write.tree(tree)
Error in kids[[parent[i]]] : 
  attempt to select less than one element in integerOneIndex

This last command is called by .to.newick in treekernel.R, and is where the error messages in #55 came from. We need to implement some check that catches malformed trees within this function and provides an informative message.

ArtPoon commented 7 years ago

One possibility is to call plot.phylo with plot=FALSE option and catch the exception:

> plot(tree, plot=FALSE)
Error in plot.phylo(tree, plot = FALSE) : 
  tree badly conformed; cannot plot. Check the edge matrix.
0ldM4j0r commented 7 years ago

552d28e Fixed issue #70 with @gtng92 help

MathiasRenaud commented 7 years ago

Plot function raises errors when branch lengths are too large (issue #82). We need to come up with another way to check if a tree is malformed.

ArtPoon commented 7 years ago

I think that our original tree string validation procedure (parsing string with ape read.tree function) was correct and that this is a problem with the plot.phylo function (when branch lengths are too long). This is not our problem! Strip out use of plot for validation.

0ldM4j0r commented 7 years ago

Removed try catch statements using the plot.phylo function 9312663111bf31c8025fcdcc7d98a3ddb67cafb2