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

Add longphase #388

Merged
merged 7 commits into from
Sep 25, 2024
Merged

Add longphase #388

merged 7 commits into from
Sep 25, 2024

Conversation

fellen31
Copy link
Collaborator

@fellen31 fellen31 commented Sep 20, 2024

This PR closes #291, by adding LongPhase as the default phaser. This reduces the runtime of the pipeline by ~3-4 hours, while producing comparable results. Both whatshap and HiPhase are kept for now, because they each have their own strengths that have not been properly examined (for example whatshap pedigree phasing).

Co-phasing of SNVs and SVs with HiPhase is removed in this PR, since that workflow was not being tested, and was not working.

All SV-callers VCFs outputs are not similar, and the only really supported caller for HiPhase is pbsv. Other callers might require modifications to the VCF, which adds complexity and needs to be investigated further. Perhaps we should always run multiple SV-callers to support this.

I've added an issue to support co-phasing of SNVs and SVs with LongPhase and/or HiPhase (#389) to track the progress of (re-implementing) co-phasing.

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
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,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).

Copy link

github-actions bot commented Sep 20, 2024

nf-core lint overall result: Passed ✅

Posted for pipeline commit 24a36b6

+| ✅ 169 tests passed       |+
#| ❔  20 tests were ignored |#

❔ Tests ignored:

  • files_exist - File is ignored: CODE_OF_CONDUCT.md
  • files_exist - File is ignored: assets/nf-core-nallo_logo_light.png
  • files_exist - File is ignored: docs/README.md
  • files_exist - File is ignored: docs/images/nf-core-nallo_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-nallo_logo_dark.png
  • files_exist - File is ignored: .github/ISSUE_TEMPLATE/config.yml
  • files_exist - File is ignored: .github/workflows/awstest.yml
  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • files_exist - File is ignored: conf/modules.config
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: CODE_OF_CONDUCT.md
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File ignored due to lint config: assets/nf-core-nallo_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-nallo_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-nallo_logo_dark.png
  • files_unchanged - File ignored due to lint config: docs/README.md
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore
  • modules_config - modules_config

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-09-25 13:33:44

@fellen31 fellen31 force-pushed the add-longphase branch 5 times, most recently from 683180d to 251acf4 Compare September 20, 2024 08:48
@fellen31 fellen31 marked this pull request as ready for review September 20, 2024 09:39
@fellen31 fellen31 requested a review from a team as a code owner September 20, 2024 09:39
Copy link
Contributor

@rannick rannick left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

README.md Outdated
--fasta <reference.fasta> \
--outdir <OUTDIR>
--preset revio \
--fasta reference.fasta \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--fasta reference.fasta \
--fasta reference.fasta \

Copy link
Contributor

Choose a reason for hiding this comment

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

I like formatting the options within brackets better as it highlights what the user has to provide

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll change it back!

@fellen31
Copy link
Collaborator Author

Thanks @rannick!

@fellen31 fellen31 merged commit c3c2ae2 into genomic-medicine-sweden:dev Sep 25, 2024
14 checks passed
@fellen31 fellen31 deleted the add-longphase branch September 25, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add longphase as the default phaser
2 participants