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

Dnascope module and subworkflow in Sarek #1193

Merged
merged 56 commits into from
Oct 4, 2023
Merged

Dnascope module and subworkflow in Sarek #1193

merged 56 commits into from
Oct 4, 2023

Conversation

asp8200
Copy link
Contributor

@asp8200 asp8200 commented Aug 28, 2023

Implementation of DNAscope. #405

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).

workflows/sarek.nf Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Aug 30, 2023

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit d6f610c

+| ✅ 147 tests passed       |+
#| ❔   8 tests were ignored |#
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in WorkflowSarek.groovy: Optionally add in-text citation tools to this list.

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-10-04 16:14:54

nextflow.config Outdated Show resolved Hide resolved
@asp8200 asp8200 changed the title DRAFT: Dnascope module and subworkflow in Sarek. Dnascope module and subworkflow in Sarek. Sep 20, 2023
@asp8200 asp8200 marked this pull request as ready for review September 20, 2023 13:49
@asp8200 asp8200 changed the title Dnascope module and subworkflow in Sarek. Dnascope module and subworkflow in Sarek Sep 20, 2023
@maxulysse
Copy link
Member

Can you check the merge conflicts?

@maxulysse
Copy link
Member

LGTM

@maxulysse
Copy link
Member

Can you sync and update CHANGELOG

@asp8200 asp8200 merged commit df6df02 into dev Oct 4, 2023
58 checks passed
@asp8200 asp8200 deleted the sentieon_dnascope branch October 4, 2023 17:14

process {

// TO-DO: duplicate!!
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need an issue for this todo?

joint_mutect2 = false // if true, enables patient-wise multi-sample somatic variant calling
only_paired_variant_calling = false // if true, skips germline variant calling for normal-paired sample
sentieon_dnascope_emit_mode = "variant" // default value for Sentieon dnascope
sentieon_dnascope_model = "https://s3.amazonaws.com/sentieon-release/other/SentieonDNAscopeModel1.1.model"
Copy link
Contributor

Choose a reason for hiding this comment

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

was this the s3 thing you guys mentioned or is this something new?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I believe Maxime is working on getting that file into igenomes.

@FriederikeHanssen
Copy link
Contributor

Great work, sorry I only got to review it now. Do you have a chance to add it to the subway map/overview image?

@asp8200
Copy link
Contributor Author

asp8200 commented Oct 11, 2023

Great work, sorry I only got to review it now. Do you have a chance to add it to the subway map/overview image?

Oh, I totally forgot about that, Rike. I'm afraid I don't know how to use those graphics programs for updating the diagrams. Perhaps you or @maxulysse could update the diagrams?

@FriederikeHanssen
Copy link
Contributor

Great work, sorry I only got to review it now. Do you have a chance to add it to the subway map/overview image?

Oh, I totally forgot about that, Rike. I'm afraid I don't know how to use those graphics programs for updating the diagrams. Perhaps you or @maxulysse could update the diagrams?

Sure. It's an alternative to haplotyper, right? When would you choose which?

@asp8200
Copy link
Contributor Author

asp8200 commented Oct 11, 2023

Yes, it is an alternative to Sentieon's haplotyper, but where as Sentieon's haplotyper was designed to give the same results at GATK's haplotypercaller - but faster, Sentieon's dnascope improves both "speed and accuracy, and it can perform structural variant calling in addition to calling SNPs and small indels."

@FriederikeHanssen
Copy link
Contributor

ok so in the plot should move it inbetween SNps/indels and svs then

@asp8200
Copy link
Contributor Author

asp8200 commented Oct 11, 2023

SNps/indels and svs

? You mean listing "Sentieon DNAscope" (or just "DNAscope") with it own bullet between these two bullets:

image

I think that would be fine.

@FriederikeHanssen
Copy link
Contributor

I was thinking more of the sbway map. Anyways I'll make a proposal in the coming days and send it your way

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.

3 participants