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.

Sign up for GitHub

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

Jump to bottom

Include new module ASCAT #606

Merged
merged 37 commits into from
Jul 1, 2022
Merged

Include new module ASCAT #606

merged 37 commits into from
Jul 1, 2022

Conversation

SusiJo
Copy link
Contributor

@SusiJo SusiJo commented Jun 24, 2022 •
edited
Loading

PR checklist

This PR addresses issue #100 and implements the allele-specific copy number caller ASCAT in sarek.

  • 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 - add to the software_versions process and a regex to scrape_software_versions.py
    • 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).
  • 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).

SusiJo added 27 commits June 21, 2022 13:53
@SusiJo
add ascat locally
7a1b0ce
@SusiJo
install unzip
636f919
@SusiJo
add ascat genome string
5bb319f
@SusiJo
add unzip
5420559
@SusiJo
adjust format
0812d16
@SusiJo
add ascat stuff
7e569cd
@SusiJo
fix channels
77618e9
@SusiJo
quick fixes
60205c3
@SusiJo
quick fixes
e311941
@SusiJo
quick fixes
3aa4c01
@SusiJo
fix new params
052c40f
@SusiJo
fix ascat_params
e2d97d5
@SusiJo
quick fixes
798a43e
@SusiJo
test empty channel
0165763
@SusiJo
quick fixes
fe40602
@SusiJo
adjust gender, add stuff to test ascat locally
110a954
@SusiJo
Merge remote-tracking branch 'upstream/dev' into ascat
762b2cc
@SusiJo
installed new module ascat
8c306eb
@SusiJo
change order of channels
438b94c
@SusiJo
fix userEmulation closure
89833a2
@SusiJo
push working version
574f7bd
@SusiJo
updated to use igenomes
79b95c0
@SusiJo
fix loci input + add warnings
dd0426d
@SusiJo
add warnings
6b60a92
@SusiJo
add download link for reference files
125e5a3
@SusiJo
Merge remote-tracking branch 'origin/dev' into ascat
3ff0eb0
@SusiJo
Merge branch 'ascat' of https://github.com/SusiJo/sarek into ascat
66cf52a
@github-actions
Copy link

github-actions bot commented Jun 24, 2022 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit f09de9a

+| ✅ 145 tests passed       |+
#| ❔   4 tests were ignored |#
!| ❗   2 tests had warnings |!

❗ Test warnings:

  • readme - README did not have a Nextflow minimum version badge.
  • schema_description - No description provided in schema for parameter: ascat_chromosomes

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: assets/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_dark.png
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy

✅ Tests passed:

Run details

  • nf-core/tools version 2.4.1
  • Run at 2022-07-01 13:25:42

@SusiJo
Copy link
Contributor Author

SusiJo commented Jun 24, 2022

This PR also fixes the docker.userEmulation closure that was not working properly and created files in the work dir with root permissions.

@SusiJo
Copy link
Contributor Author

SusiJo commented Jun 24, 2022

Let me know if i should add (uncommented) output files to the test.yml files. However, please note that unfortunately they cannot be run, since the testdata is too large for GHA.

@SusiJo
Copy link
Contributor Author

SusiJo commented Jun 24, 2022

Also please let me know if the ASCAT specific parameters min_base_qual, min_map_qual, skip_allele_counting_tumour, skip_allele_counting_normal should be added as new parameters to the pipeline as well.

@FriederikeHanssen
Copy link
Contributor

Let me know if i should add (uncommented) output files to the test.yml files. However, please note that unfortunately they cannot be run, since the testdata is too large for GHA.

Yes that would be great. Can you maybe a add a new file test_tools_manually that contain all the tests that should be run manually? ( so also the standard mutect test that fails)? Then we can get gha finally back to completely finish again. (The tags must be exclusive then though)

FriederikeHanssen
FriederikeHanssen requested changes Jun 27, 2022
View reviewed changes
Copy link
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.

Really great work!

conf/modules.config Outdated Show resolved Hide resolved
conf/modules.config Outdated Show resolved Hide resolved
nextflow.config Show resolved Hide resolved
@FriederikeHanssen
Copy link
Contributor

Also congrats on being the first person with a fully green test suite 😆 🎉

@SusiJo SusiJo marked this pull request as ready for review June 29, 2022 11:25
@SusiJo SusiJo requested a review from maxulysse as a code owner June 29, 2022 11:25
@FriederikeHanssen FriederikeHanssen merged commit 340f813 into nf-core:dev Jul 1, 2022
This was referenced Jul 1, 2022
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FriederikeHanssen FriederikeHanssen FriederikeHanssen approved these changes

@maxulysse maxulysse Awaiting requested review from maxulysse maxulysse is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SusiJo @FriederikeHanssen