Skip to content

add --aligner to choose between bwa-mem and bwa-mem2#282

Merged
maxulysse merged 5 commits intonf-core:devfrom
maxulysse:bwa-mem-all
Sep 21, 2020
Merged

add --aligner to choose between bwa-mem and bwa-mem2#282
maxulysse merged 5 commits intonf-core:devfrom
maxulysse:bwa-mem-all

Conversation

@maxulysse
Copy link
Copy Markdown
Member

Struggling a bit to compute a conda env on my computer, with bwa and bwa-mem2, but also with either one of them.
So finally, opposed to what I said on Slack, I'm trying to have both of them at once, and see if it build on dockerhub.

nf-core/sarek pull request

Many thanks for contributing to nf-core/sarek!

Please fill in the appropriate checklist below (delete whatever is not relevant).
These are the most common things requested on pull requests (PRs).

PR checklist

  • This comment contains a description of changes (with reason)
  • If you've fixed a bug or added code that should be tested, add tests!
  • If necessary, also make a PR on the nf-core/sarek branch on the nf-core/test-datasets repo
  • Ensure the test suite passes (nextflow run . -profile test,docker).
  • Make sure your code lints (nf-core lint .).
  • Documentation in docs is updated
  • CHANGELOG.md is updated
  • README.md is updated

Learn more about contributing: CONTRIBUTING.md

@FriederikeHanssen
Copy link
Copy Markdown
Contributor

Maybe I missed it, but if bwa-mem2 is set for the --aligner, shouldn't then params.bwa=false be set by default, to avoid the indices mess? Or are you going to set them all in igenomes after all?

@maxulysse
Copy link
Copy Markdown
Member Author

maxulysse commented Sep 18, 2020

Default would be bwa-mem currently, so no issue with the current indices:

aligner = 'bwa-mem'

@FriederikeHanssen
Copy link
Copy Markdown
Contributor

OK, but if I set --aligner-bwa-mem2, then I have to set bwa=false, too, right? Maybe this can be already included, if this aligner is chosen?

@maxulysse
Copy link
Copy Markdown
Member Author

That's a very good point.
I'll add that to the doc right away

Copy link
Copy Markdown
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bwa changes look good. can't say much about the json schema. For me it looks good, but I haven't played around with it so far.

@maxulysse
Copy link
Copy Markdown
Member Author

I know now that the conda env takes around 3 hours to compute, so trying again on my side to see if it all works well.
Hopefully can merge that today

@maxulysse maxulysse merged commit 13e4815 into nf-core:dev Sep 21, 2020
@maxulysse maxulysse deleted the bwa-mem-all branch September 21, 2020 12:36
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.

2 participants