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.

Sign up for GitHub

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

Jump to bottom

Duplicate sample names in tree #72

Open
michellescribner opened this issue Dec 5, 2023 · 3 comments
Open

Duplicate sample names in tree #72

michellescribner opened this issue Dec 5, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@michellescribner
Copy link

Current Behavior

Hello! I am encountering an issue when attempting to drag and drop this tree into auspice.us. Samples are appearing in duplicate with "_2" appended to the end. I drag and dropped the file with a ".nwk" file ending but changed the ending to ".txt" to attach to this issue.

test-tree-duplicate.txt

image

Expected behavior

Sample names should only appear once. There should be 10 samples total.

How to reproduce

Steps to reproduce the current behavior:
Drag and drop file above into auspice.us.

Your environment: if browsing Nextstrain online

  • Operating system: Windows
  • Browser: Chrome

Additional context

Issue is not occurring in other tree visualization tools (iTol, MicroReact).

Thank you!
@michellescribner michellescribner added the bug Something isn't working label Dec 5, 2023
@jameshadfield
Copy link
Member

Oh wow. I can't explain what's going on here but the underlying issue is also caused by the recent merge of #69 (i.e. the same as your earlier issue #71). It's not just duplicate names, the entire tree topology is wrong. Here's how it should look:

image

jameshadfield added a commit that referenced this issue Dec 6, 2023
@jameshadfield
WIP Revert "Improve Newick parsing"
669f170 
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
@jameshadfield
Revert "Improve Newick parsing"
c95c007 
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
@jameshadfield
Revert "Improve Newick parsing"
e42e75c 
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
Copy link
Member

This tree should now be correctly displayed in auspice.us (via #73)

@michellescribner
Copy link
Author

Thank you so much! We are now seeing the tree correctly displayed! We really appreciate your quick work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jameshadfield @michellescribner