-
Notifications
You must be signed in to change notification settings - Fork 447
Add checks for uniqueness of sample ids #1902
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
base: dev
Are you sure you want to change the base?
Conversation
[meta.patient, meta.subMap('sample', 'status')] | ||
} | ||
.unique() | ||
.groupTuple() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's double check here that we are not collapsing out lane
@@ -39,7 +59,7 @@ nextflow_pipeline { | |||
assert workflow.failed | |||
assertAll( | |||
{ assert snapshot( | |||
workflow.stderr.toString().replace("[", "").replace("]", "").split(",")[0] | |||
workflow.stderr.toString().contains("Patient [test] has more than one sample [2] with normal status [0] and one sample with tumor status [1].") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why changing this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for me this was inconsitent across the nextflow versions so that I always got a mismatch
[ | ||
"\u001b0;31mThe following invalid input values have been detected:", | ||
" ", | ||
" \t-> Entry 2: Error for field 'sample' (test 2): \"test 2\" does not match regular expression ^\\S+$ (Sample ID must be provided", | ||
" cannot contain spaces and must be a string value)", | ||
" \u001b0m" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked that we were capturing the stderr 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for me this was inconsitent across the nextflow versions so that I always got a mismatch
PR checklist
Closes #1674
nf-core pipelines lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).nextflow run . -profile debug,test,docker --outdir <OUTDIR>
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).