-
Notifications
You must be signed in to change notification settings - Fork 107
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
udpating to use gofasta rather than datafunk as a speedup for alignme…
…nt. Also piping sam from minimap2 to gofasta now rather than writing to a file
- Loading branch information
Showing
2 changed files
with
17 additions
and
31 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3617ac1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the switch to
gofasta
may break some environments: we had a working pangolin installation, then updated usingpangolin --update
, and now it didn't work anymore, with no descriptive error. We had to dig through the verbose messages to find the "command not found" error3617ac1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @niemasd, I don't think it'll 'break' the environment per se, I think they'll just have to be updated. I'll clarify in the docs that the
--update
flag only updates to latest versions of pangolin and pangoLEARN, doesn't deal with dependencies. If you follow the alternative update instructions (https://cov-lineages.org/pangolin_docs/updating.html) it should update the environment successfully! If there's problems with that and gofasta won't install for some reason, just let me know!3617ac1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying! Perhaps one workaround would be to have checks for the dependencies at the beginning of running pangolin.
The primary issue was that the error message we received was about minimap2 crashing (with no other info), and when we tried running the minimap2 command, minimap2 complained that the FASTA file it was looking for didn't exist. We then reran pangolin in verbose mode and had to dig through the verbose output to notice the
gofasta: command not found
error. I coincidentally happened to also be looking at the pangolin GitHub repo and saw that the most recent commit mentioned switching to gofasta, which prompted me to think that perhaps a new dependency was introduced.If pangolin were to do a quick dependency check at the beginning and provide feedback regarding what tool is missing (e.g. "Dependency gofasta does not seem to be installed. Please refer to the Pangolin installation instructions to view how to install it") or something, I think it could potentially help. Here are some examples of how I do this type of dependency check in my ViralMSA tool:
https://github.com/niemasd/ViralMSA/blob/master/ViralMSA.py#L119
Thanks for the awesome tool, and I hope you're doing well! 😄