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

Refactor Variant Calling #497

Merged
merged 54 commits into from
Mar 24, 2022
Merged

Conversation

FriederikeHanssen
Copy link
Contributor

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

@FriederikeHanssen FriederikeHanssen changed the title Add CNV Refactor Variant Calling Mar 21, 2022
@FriederikeHanssen
Copy link
Contributor Author

@maxulysse almost happy with the state of this, just need to add haplotypecaller and mutect back in. I think i am going for smaller PRs now, makes both of our lifes a lot easier I think.

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.

<3

@FriederikeHanssen
Copy link
Contributor Author

FriederikeHanssen commented Mar 23, 2022

ok, Haplotypecaller will be fixed in the next PR, after the discussion on slack it will be a little bit more then just refactoring. But it seems straight forward in the next one now.

For Deepvariant: The gvcf are generated no matter what, I don't see why we shouldn't publish them, so i removed it for now.

I don't know whats going on with all the CI tst, locally it's fine:

❯ NF_CORE_MODULES_TEST=1 TMPDIR=~ PROFILE=docker pytest --tag default --symlink --keep-workflow-wd
============================================================================================================================ test session starts ============================================================================================================================
platform darwin -- Python 3.9.10, pytest-7.1.1, pluggy-1.0.0
rootdir: /Users/monarchy/Projects/Coding/sarek
plugins: workflow-1.6.0
collecting ... 
collected 17 items                                                                                                                                                                                                                                                          

Run default pipeline:
        command:   nextflow run main.nf -profile test,docker
        directory: /Users/monarchy/pytest_workflow_tewepav2/Run_default_pipeline
        stdout:    /Users/monarchy/pytest_workflow_tewepav2/Run_default_pipeline/log.out
        stderr:    /Users/monarchy/pytest_workflow_tewepav2/Run_default_pipeline/log.err
'Run default pipeline' done.

tests/test_default.yml .................                                                                                                                                                                                                                              [100%] Keeping temporary directories and logs. Use '--kwd' or '--keep-workflow-wd' to disable this behaviour.

Edit: Set ressource requests to high in test.config to speed up local computation 🤦‍♀️

@FriederikeHanssen FriederikeHanssen marked this pull request as ready for review March 23, 2022 22:09
@maxulysse
Copy link
Member

tests/test_default.yml .................

did you empty your results folder before doing the tests?

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

See edit: increased resources locally to speed up tests and accidentally pushed them. They ran now after reducing them again

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.

<3

@FriederikeHanssen FriederikeHanssen merged commit 9d74728 into nf-core:dev Mar 24, 2022
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