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

fix Ascat R script execution #127

Merged
merged 15 commits into from
Feb 25, 2020
Merged

fix Ascat R script execution #127

merged 15 commits into from
Feb 25, 2020

Conversation

ggabernet
Copy link
Member

Fixes Ascat R script executions

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

Copy link
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

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

Looks good, I'm just waiting if it works on our server as well, I'll likely merge it tomorrow

bin/run_ascat.r Outdated Show resolved Hide resolved
@maxulysse
Copy link
Member

It would be good to finally manage to make files so that we could CI the ASCAT processes as well.

@maxulysse
Copy link
Member

While we're at it, may I suggest we improve the ASCAT process with:

    purity_ploidy = (params.ascat_purity && params.ascat_ploidy) ? "--purity ${params.ascat_purity} --ploidy ${params.ascat_ploidy}" : ""
    """
    for f in *BAF *LogR; do sed 's/chr//g' \$f > tmpFile; mv tmpFile \$f;done
    run_ascat.r --tumorbaf ${bafTumor} --tumorlogr ${logrTumor} --normalbaf ${bafNormal} --normallogr ${logrNormal} --tumorname ${idSampleTumor} --basedir ${baseDir} --gcfile ${acLociGC} --gender ${gender} ${purity_ploidy}
    """

to replace

sarek/main.nf

Lines 2598 to 2611 in d1a5691

ascat_purity=params.ascat_purity
ascat_ploidy=params.ascat_ploidy
if (params.ascat_purity && params.ascat_ploidy)
"""
for f in *BAF *LogR; do sed 's/chr//g' \$f > tmpFile; mv tmpFile \$f;done
Rscript ${workflow.projectDir}/bin/run_ascat.r --tumorbaf ${bafTumor} --tumorlogr ${logrTumor} --normalbaf ${bafNormal} --normallogr ${logrNormal} --tumorname ${idSampleTumor} --basedir ${baseDir} --gcfile ${acLociGC} --gender ${gender} --purity ${ascat_purity} --ploidy ${ascat_ploidy}
"""
else
"""
for f in *BAF *LogR; do sed 's/chr//g' \$f > tmpFile; mv tmpFile \$f;done
Rscript ${workflow.projectDir}/bin/run_ascat.r --tumorbaf ${bafTumor} --tumorlogr ${logrTumor} --normalbaf ${bafNormal} --normallogr ${logrNormal} --tumorname ${idSampleTumor} --basedir ${baseDir} --gcfile ${acLociGC} --gender ${gender}
"""

@maxulysse
Copy link
Member

But I could do that in a separate PR if you prefer

@ggabernet
Copy link
Member Author

It's all good, however you like, let me just test first if the param name thing fixed the issue, then I can add it here

@ggabernet
Copy link
Member Author

And I can also help with adding tests for ASCAT, if you like

@maxulysse
Copy link
Member

And I can also help with adding tests for ASCAT, if you like

That would be amazing if we could do that

@maxulysse
Copy link
Member

Worked on my side, with the _gc fix and the new purity_ploidy updated command.

@maxulysse maxulysse added this to the 2.6 milestone Feb 25, 2020
@ggabernet
Copy link
Member Author

ok just added your suggestion

main.nf Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@maxulysse maxulysse merged commit 0695a4a into nf-core:dev Feb 25, 2020
@maxulysse maxulysse mentioned this pull request Mar 1, 2020
8 tasks
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