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.

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

sentieon-bwamem #994

Merged
merged 12 commits into from
Apr 13, 2023
Merged

sentieon-bwamem #994

merged 12 commits into from
Apr 13, 2023

Conversation

asp8200
Copy link
Contributor

@asp8200 asp8200 commented Apr 13, 2023

The new CI-tests for sentieon/bwamem are now passing.

Let me know what you think.

TO-DO: Update the changelog.

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 you've added a new tool - have you followed the pipeline conventions in the contribution docs- [ ] If necessary, also make a PR on the nf-core/sarek branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@asp8200 asp8200 requested a review from maxulysse as a code owner April 13, 2023 07:39
@maxulysse maxulysse mentioned this pull request Apr 13, 2023
9 tasks
@github-actions
Copy link

github-actions bot commented Apr 13, 2023

nf-core lint overall result: Passed ✅

Posted for pipeline commit cfab8d8

+| ✅ 151 tests passed       |+
#| ❔   9 tests were ignored |#

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.7.2
  • Run at 2023-04-13 14:07:47

tests/config/tags.yml Outdated Show resolved Hide resolved
@@ -335,10 +335,13 @@ workflow SAREK {
bwa = params.bwa ? Channel.fromPath(params.bwa).collect() : PREPARE_GENOME.out.bwa
bwamem2 = params.bwamem2 ? Channel.fromPath(params.bwamem2).collect() : PREPARE_GENOME.out.bwamem2
dragmap = params.dragmap ? Channel.fromPath(params.dragmap).collect() : PREPARE_GENOME.out.hashtable
sentieon = params.sentieon ? Channel.fromPath(params.sentieon).collect() : PREPARE_GENOME.out.sentieon
Copy link
Member

Choose a reason for hiding this comment

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

we should use params.bwa for that, the way I see it params.sentieon should only be a flag to allow usage of aligner sentieon-bwamem and the extra sentieon tools

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I understand why we need params.sentieon.

Copy link
Contributor

Choose a reason for hiding this comment

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

basically select that you want to use bwa but the sentieon implementaion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but for that I would specify --aligner sentieon-bwamem

workflows/sarek.nf Outdated Show resolved Hide resolved
.github/workflows/pytest-workflow.yml Outdated Show resolved Hide resolved
conf/modules/prepare_genome.config Outdated Show resolved Hide resolved
workflows/sarek.nf Outdated Show resolved Hide resolved
@asp8200 asp8200 changed the title DRAFT: sentieon-bwamem sentieon-bwamem Apr 13, 2023
@maxulysse maxulysse changed the base branch from dev to sentieon April 13, 2023 14:47
@maxulysse
Copy link
Member

I created a new sentieon branch from dev so that we can merge all sentieon related stuff all at once in dev once we're happy.

@maxulysse
Copy link
Member

I'm thinking we might need to have do a release before we actually finish sentieon, so just to be on the safe side

@maxulysse maxulysse merged commit 543ef58 into sentieon Apr 13, 2023
@maxulysse maxulysse deleted the add_sentieon_bwamem branch April 13, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants