Auspice has no problem reading a broken nwk tree #66
Labels
bug
Something isn't working
Comments
|
parseNewick is the custom function for parsing a Newick string to a Javascript object. Looks like it currently does not do any sort of validation of the Newick format. |
jameshadfield
added a commit
that referenced
this issue
Dec 6, 2023
Two recent issues (#71, #72) provide examples where the improved parsing either didn't parse a valid newick tree or (much more worryingly) returned an entirely incorrect tree structure, including nodes not present in the newick. See those issues for details, including the tree files. While this reversion will re-introduce bugs such as #66 and the bug in <https://discussion.nextstrain.org/t/displaying-trees-from-ncbi-pathogen-browser-in-auspice-us/1456/4>, but they are lesser than the bugs introduced by #69. This reverts commit cabba98, although subsequent changes to package-lock.json mean it's not a clean revert.
jameshadfield
added a commit
that referenced
this issue
Dec 6, 2023
The newick parser we use is known to have issues with quoted node names, as it will parse the node name content as newick. As a quick fix, disallow trees with quotes. The result is that the trees in #66 and <https://discussion.nextstrain.org/t/displaying-trees-from-ncbi-pathogen-browser-in-auspice-us/1456/4> will now result in an error. The downside is trees with quoted names but no newick-like structure within the node name will also fail to load.
jameshadfield
added a commit
that referenced
this issue
Dec 6, 2023
Two recent issues (#71, #72) provide examples where the improved parsing either didn't parse a valid newick tree or (much more worryingly) returned an entirely incorrect tree structure, including nodes not present in the newick. See those issues for details, including the tree files. While this reversion will re-introduce bugs such as #66 and the bug in <https://discussion.nextstrain.org/t/displaying-trees-from-ncbi-pathogen-browser-in-auspice-us/1456/4>, but they are lesser than the bugs introduced by #69. This reverts commit cabba98, although subsequent changes to package-lock.json mean it's not a clean revert.
jameshadfield
added a commit
that referenced
this issue
Dec 6, 2023
The newick parser we use is known to have issues with quoted node names, as it will parse the node name content as newick. As a quick fix, disallow trees with quotes. The result is that the trees in #66 and <https://discussion.nextstrain.org/t/displaying-trees-from-ncbi-pathogen-browser-in-auspice-us/1456/4> will now result in an error. The downside is trees with quoted names but no newick-like structure within the node name will also fail to load.
jameshadfield
added a commit
that referenced
this issue
Dec 6, 2023
Two recent issues (#71, #72) provide examples where the improved parsing either didn't parse a valid newick tree or (much more worryingly) returned an entirely incorrect tree structure, including nodes not present in the newick. See those issues for details, including the tree files. While this reversion will re-introduce bugs such as #66 and the bug in <https://discussion.nextstrain.org/t/displaying-trees-from-ncbi-pathogen-browser-in-auspice-us/1456/4>, but they are lesser than the bugs introduced by #69. This reverts commit cabba98, although subsequent changes to package-lock.json mean it's not a clean revert.
jameshadfield
added a commit
that referenced
this issue
Dec 6, 2023
The newick parser we use is known to have issues with quoted node names, as it will parse the node name content as newick. As a quick fix, disallow trees with quotes. The result is that the trees in #66 and <https://discussion.nextstrain.org/t/displaying-trees-from-ncbi-pathogen-browser-in-auspice-us/1456/4> will now result in an error. The downside is trees with quoted names but no newick-like structure within the node name will also fail to load.
|
As of #73 this file will no longer parse because it has single quotes (acting as an apostrophe) in node names, but the underlying unbalanced parentheses bug isn't fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Current Behavior
Fascinatingly, Auspice has no issue reading a broken nwk tree with unmatching parentheses.
When viewing the raw nwk tree in Auspice it didn't complain. Only when I ran it through
augur exportdid I get a complaint.It's not so much a bug as I'm surprised that we're not even logging anything to the console.
Expected behavior
When I give a broken nwk, Auspice warns me.
How to reproduce
Steps to reproduce the current behavior:
The text was updated successfully, but these errors were encountered: