-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added support for kraken2 #75
Conversation
Added support for kraken2. Multiple libraries are supported for minimap2 as well.
Pull Request Test Coverage Report for Build 2198481599Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
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.
Thank you @charles-cowart, looks good, a few question and I think we are missing a test for when the minimap2 command has more than 2 references, no? ... The test can be a direct call to _gen_chained_cmd, just to make sure that they are generated fine and we don't mess something up with future changes.
f' sr -t {self.nprocs} {self.mmi_db_path} - -a | ' | ||
f'{self.samtools_path} fastq -@ {self.nprocs} -f 12 -F ' | ||
f'256 -1 {path1} -2 {path2}') | ||
result += (f' -l 100 -i {read1_input_path} -I {read2_input_path} -w ' |
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.
@charles-cowar, could you open an issue to define the min length of the reads via the config file or somewhere else that is not hardcoded? Thank you.
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.
Done! #76
print("LAST LINE EXPECTED ##################") | ||
print(config['last_line']) | ||
print("##################") | ||
print("LAST LINE: %s" % lines_obs[-1]) |
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 is fine to also remove the last print, no?
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.
definitely. done!
Added unittest for performing multiple minimap2 passes and kraken2.
@antgonza unitest created for supplying multiple databases to minimap2 and kraken2 |
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.
Looks good, thank you.
Added support for kraken2. Multiple libraries are supported for minimap2 as well.