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

write.tree() may export duplicate internal node labels without notification #113

Closed
stitam opened this issue Mar 22, 2024 · 3 comments · Fixed by #115
Closed

write.tree() may export duplicate internal node labels without notification #113

stitam opened this issue Mar 22, 2024 · 3 comments · Fixed by #115

Comments

@stitam
Copy link

stitam commented Mar 22, 2024

Hi,

My issue is the following: It may happen that a tree object ends up with less internal node labels than the number of internal nodes. When I export such an object with write.tree(), the function will start recycling internal node names, leading to duplicated internal node labels:

# create a random tree
set.seed(0)
tree <- ape::rtree(10)
# define node labels
tree$node.label <- paste0("Node_", 1:(tree$Nnode-2))
tree$node.label
#> [1] "Node_1" "Node_2" "Node_3" "Node_4" "Node_5" "Node_6" "Node_7"
# export and import
ape::write.tree(tree, file = "tree.nwk")
tree2 <- ape::read.tree("tree.nwk")
tree2$node.label
#> [1] "Node_1" "Node_2" "Node_3" "Node_4" "Node_5" "Node_6" "Node_7" "Node_1"
#> [9] "Node_2"

Created on 2024-03-22 with reprex v2.1.0

Linked issue providing an example where such a tree is accidentally generated: ms609/TreeTools#149.

For the sake of addressing this issue from multiple directions, I wonder if write.tree() could be updated to at least give a warning message when node names are being recycled (stopping with an informative error would be even better). Do you think this is reasonable? Many thanks.

@emmanuelparadis
Copy link
Owner

Hi,
Have you tried:

R> tree <- makeNodeLabel(tree, prefix = "Node_")
R> tree$node.label
[1] "Node_1" "Node_2" "Node_3" "Node_4" "Node_5" "Node_6"
[7] "Node_7" "Node_8" "Node_9"

?

@ms609
Copy link
Contributor

ms609 commented Mar 28, 2024

I think the issue here is the behaviour of write.tree() when a tree has been created by another (potentially erroneous) process, such that length(tree$node.label) < tree$Nnode.

I've offered a possible solution at #115.

@stitam
Copy link
Author

stitam commented Apr 2, 2024

Thanks @emmanuelparadis! Once I discovered the root of the problem I could fix it easily; I opened the issues because I thought it might be better for the long term to highlight/eliminate the inconsistency. Thanks @ms609 I think an informative warning like that is a good solution from ape's side; It is probably not within ape's scope to fix an object that was created by another package.

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 a pull request may close this issue.

3 participants