Automatically assign sex if unknown #148
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
rannick
approved these changes
May 15, 2024
| @@ -1,3 +1,3 @@ | |||
| sample,file,family_id,paternal_id,maternal_id,sex,phenotype | |||
| sample_1,/path/to/fastq_or_bam/files/sample_1.fastq.gz,FAM,PAT,MAT,1,1 | |||
There was a problem hiding this comment.
Would it make sense to have 2 tests samplesheet, one with and one without the sex assigned to test all logical gates? Maybe just a stub run. Just throwing ideas
There was a problem hiding this comment.
Absolutely! And I have, but you can't see it here.
I removed the test data and samplesheets from this repository, and put it in the nallo branch of the GMS fork of test-datasets to make it more similar to how it's handled in for example raredisease and rnafusion. I made a PR to update the test data and samplesheets used (the multisample test now includes one sample where sex is set to 0).
But maybe that is problematic, because:
- The changes made are not recorded here
- Updating the test data might break the master test profile
Do you have a suggestion for the best solution here? @jemten?
- I could change the links in master & dev to point to specific commits for each file
- I could move the samplesheets back to this repo, but will it work smoothly when merging dev to master?
I'm sure I will have to continue to refine the test data.
There was a problem hiding this comment.
I think that it is a good idea to hav the specific commit hash in the url. As you say that way we could have a different sample sheet in master and dev.
There was a problem hiding this comment.
Might make a patch to master then.
| "--secondary=no", | ||
| "-Y", | ||
| "-R @RG\\\\tID:${meta.id}\\\\tSM:${meta.id}" | ||
| ].join(' ') } |
There was a problem hiding this comment.
To me, this does not seem to relate the to sex check, so I would add a line to the changelog about it
There was a problem hiding this comment.
Adding RG was necessary for somalier, but I see your point as the change is broad. Will add to changelog!
peterpru
reviewed
May 15, 2024
Sure, I could use INFER since that is the flag supplied to somalier relate. |
jemten
reviewed
May 15, 2024
nextflow_schema.json
Outdated
| "somalier_sites": { | ||
| "type": "string", | ||
| "description": "A VCF of known polymorphic sites", | ||
| "format": "file-path" |
There was a problem hiding this comment.
would be nice to add some more constrains here. Like:
"pattern": "^\\S+\\.vcf(\\.gz)?$"
subworkflows/local/bam_assign_sex.nf
Outdated
| family_id : meta.family_id, | ||
| paternal_id : meta.paternal_id, | ||
| maternal_id : meta.maternal_id, | ||
| sex : meta2.infered_sex.toInteger(), // Use sex from somalier, only updated if sex is unknown |
There was a problem hiding this comment.
Wouldn't this always update sex with the inferred sex from somalier?
subworkflows/local/bam_assign_sex.nf
Outdated
| .map { it -> | ||
| new_meta = [ | ||
| sample_id: it.sample_id, | ||
| infered_sex: it.sex, |
There was a problem hiding this comment.
Suggested change
| infered_sex: it.sex, | |
| inferred_sex: it.sex, |
subworkflows/local/bam_assign_sex.nf
Outdated
| family_id : meta.family_id, | ||
| paternal_id : meta.paternal_id, | ||
| maternal_id : meta.maternal_id, | ||
| sex : meta2.infered_sex.toInteger(), // Use sex from somalier, only updated if sex is unknown |
There was a problem hiding this comment.
Suggested change
| sex : meta2.infered_sex.toInteger(), // Use sex from somalier, only updated if sex is unknown | |
| sex : meta2.inferred_sex.toInteger(), // Use sex from somalier, only updated if sex is unknown |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist
nf-core lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).