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

keeping intervals param deafult for all cases in GetPileupSummaries #314

Closed
wants to merge 1 commit into from

Conversation

sk-sahu
Copy link
Member

@sk-sahu sk-sahu commented Dec 13, 2020

nf-core/sarek pull request

This PR address issue #299

if no interval is provided, all the variants will be used in the same interval param.

Based on, this doc -

Although the sites (-L) and variants (-V) resources will often be identical, this need not be the case.

If a subset of variants is being used then the interval bed file can come into the picture.

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!
  • Ensure the test suite passes (nextflow run . -profile test,docker).
  • Make sure your code lints (nf-core lint .).

Learn more about contributing: CONTRIBUTING.md

@sk-sahu sk-sahu requested a review from maxulysse as a code owner December 13, 2020 16:51
@maxulysse
Copy link
Member

@szilvajuhos What do you think of this suggestion?

@szilvajuhos
Copy link
Contributor

My only issue is that we must test it before pulling it into dev. Or @sk-sahu have you tested already these?

@maxulysse
Copy link
Member

I'm a bit concerned about using the germlineResource as a source for intervals when we have no intervals.
I'm thinking it might be safer to actually use the fasta source reference file (or something like the fasta.fai)

@FriederikeHanssen FriederikeHanssen added gatk question Further information is requested labels Jul 15, 2021
@maxulysse maxulysse closed this Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants