Skip to content

Newick importer parity - #8

Open
bredelings wants to merge 9 commits into
rambaut:masterfrom
bredelings:newick-importer-parity
Open

Newick importer parity#8
bredelings wants to merge 9 commits into
rambaut:masterfrom
bredelings:newick-importer-parity

Conversation

@bredelings

@bredelings bredelings commented Aug 10, 2026

Copy link
Copy Markdown

Currently the Newick parser fails to handle some things that the NEXUS parser does handle. The main one is Newick comments -- the Newick parser currently does quite weird things when the trees have comments. This makes it hard to use Newick trees in FigTree.

The PR tries to fix some of these by making the Newick importer behave more like the NEXUS importer.

  • handle node / branch attributes in Newick comments.
  • handle tree-level attributes in Newick comments -- i.e. [&U].
  • do not set missing Newick branch lengths to 1.0
  • trim Newick internal labels
  • parse the final root branch and require the tree to end in ;
  • allow the tree to be a CompactRootedTree

This doesn't fix all issues with Newick parsing, just issues that are handled OK when parsing NEXUS. Parsing of tip labels with spaces is still not what I would expect, for example.

Node and branch attributes are treated the same, instead of attaching some on nodes and some on branches. This retains the status quo.

NOTE: The code here was written by Codex. I looked over it, and it looks reasonable, but it might still have some flaws. The design was by me though -- I didn't just tell Codex "fix this" and upload the result. So I think its worth reviewing. Also I tried it and it seems to work. I do hope we can get this included in FigTree -- I really hate treating internal node names as posterior probabilities, and this should mean I don't have to do that.

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 this pull request may close these issues.

1 participant