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

strandRule unnecessary/unused in process rseqc #119

Closed
andreas-wilm opened this issue Nov 13, 2018 · 3 comments
Closed

strandRule unnecessary/unused in process rseqc #119

andreas-wilm opened this issue Nov 13, 2018 · 3 comments

Comments

@andreas-wilm
Copy link

Hello, looks like strandRule is computed unnecessarily or is at least unused in process rseqc

Best,
Andreas

@ewels
Copy link
Member

ewels commented Nov 21, 2018

Hah, correct. It looks like we used to use it for the RPKM_saturation.py command:

rnaseq/main.nf

Line 644 in 5d65867

RPKM_saturation.py -i $bam_rseqc -r $bed12 $strandRule -o ${bam_rseqc.baseName}.RPKM_saturation

We then later removed this call: f6f6669#diff-28b7bea57b04f15beeb4c1de0ee78382L725

I'm not totally sure whether this removal was intentional or not. The rest of the commit is where we first moved geneBody_coverage.py out into its own process, so we were definitely thinking about performance issues at the time. My guess is that I said that we should remove that command because it was super slow and performance-hogging and we already had very similar QC metrics from dupRadar.

Anyway, yes I think you're right that we can remove these lines without any effect:

rnaseq/main.nf

Lines 732 to 737 in 1cd5ab7

def strandRule = ''
if (forward_stranded && !unstranded){
strandRule = params.singleEnd ? '-d ++,--' : '-d 1++,1--,2+-,2-+'
} else if (reverse_stranded && !unstranded){
strandRule = params.singleEnd ? '-d +-,-+' : '-d 1+-,1-+,2++,2--'
}

Note that it won't change the way that the pipeline runs or is configured though, as the strandedness configuration is still used in other processes.

Phil

@ewels
Copy link
Member

ewels commented Nov 21, 2018

Oops, accidentally auto-closed the issue from my commit. Should wait for the PR to be merged really.

@apeltzer
Copy link
Member

Done :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants