-
Notifications
You must be signed in to change notification settings - Fork 199
SmartSeq - Bugfixes #486
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?
SmartSeq - Bugfixes #486
Conversation
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 3.2.0. For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation. |
Hi @sminot, thanks for the PR! SmartSeq support has been frequently requested and until now we suggested to use nf-core/rnaseq instead, but I can see how the AnnData/Seurat conversion etc. is useful. is this for SmartSeq2 specifically? Because I think SmartSeq3 also requires UMI deduplication. |
Hi @grst, Yes, the debugging I did here was on a SmartSeq2 dataset. But hopefully the modifications are still useful even if users provide the appropriate parameters for SmartSeq3 data as well? |
if [[ "$protocol" == "SmartSeq" ]]; then | ||
echo "SmartSeq protocol detected, setting --soloUMIdedup to NoDedup." >&2 | ||
soloUMIdedupArg="--soloUMIdedup NoDedup" |
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.
But hopefully the modifications are still useful even if users provide the appropriate parameters for SmartSeq3 data as well?
I was specifically refering to this statement. For SmartSeq3 UMI deduplication should be enabled.
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.
My apologies, I should have realized that. I agree that the UMI deduplication should be able to enabled by the user. I will leave it to you whether that should be done by setting the protocol param (differentiating SmartSeq2 and SmartSeq3), or simply letting the UMI deduplication be a separate parameter. I can implement if you advise on your preferred approach.
PR #488 has additionally just appeared, which tries to achieve something similar. I have to think how to harmonize them. |
While using this pipeline to analyze some SmartSeq data, I ran into a couple of issues that needed to be addressed. This PR contains fixes for a small number of minor issues:
PR checklist
nf-core pipelines lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).nextflow run . -profile debug,test,docker --outdir <OUTDIR>
).CHANGELOG.md
is updated.