Fix bugs; implement SNP workflow #61
Conversation
There was a problem hiding this comment.
Thanks a lot for this PR! There are some good catches and the highly demanded version option. Since this are multiple things (and maybe we need some additional work before release) can you make a PR to the development branch and have a look at my review comments here?
But thanks a lot again!
| process { | ||
| withLabel: longtime { | ||
| executor='slurm' | ||
| //queue='default' | ||
| clusterOptions = '-t 3-00:00:0 --mem=6G' | ||
| } | ||
| withLabel: shorttime { | ||
| executor='slurm' | ||
| //queue='default' | ||
| clusterOptions = '-t 00-01:00:0 --mem=6G' | ||
| } | ||
| withLabel: highmem { | ||
| executor='slurm' | ||
| queue='bigmem' | ||
| clusterOptions = '-t 00-20:00:0 --mem=80G' | ||
| } | ||
| } |
There was a problem hiding this comment.
can you revert this changes?
I think it is better to to leave the default without any cluster environment.
It is also documented like this in the docs
| @@ -1,5 +1,5 @@ | |||
| //MPRAflow version | |||
| params.version="2.3.4" | |||
| params.version="2.3.5" | |||
There was a problem hiding this comment.
I think PR has multiple thinks (fixes and features) in it. So due to (semantic versioning)[https://semver.org/lang/de/] it is not a increase of the patch version.
Can you redirect the PR to the development branch? I think we have to do some steps (update documentation, fix some issues you found (hard coded BC length)) before creating a new release.
| - tk=8.6.8=hbc83047_0 | ||
| - wheel=0.33.6=py27_0 | ||
| - xz=5.2.4=h14c3975_4 | ||
| - zlib=1.2.11=h7b6447c_3 |
There was a problem hiding this comment.
Is there a reason for this change?
Otherwiese I would leav eit as it is.
| - nextflow=20.01 | ||
| - nextflow=20.10 | ||
| - samtools | ||
| - conda<4.7 |
There was a problem hiding this comment.
Any reason for teh NF increase and especially adding the other two software packages?
samtools should be handled by the environmen files I guess. Conda I don't know. do you have issues with it?
| @@ -111,7 +111,7 @@ | |||
| counts.insert(0, 'Label', label) | |||
| #print(counts) | |||
|
|
|||
| mask=(counts['Barcode'].str.len() == 15) | |||
| mask=(counts['Barcode'].str.len() == 16) # Scott change - hardcoded barcode length is a bad idea, but so is spending a year refactoring this. | |||
There was a problem hiding this comment.
Oh that's crazy. thanks for the catch! But we should ass teh BC length via the options. In nexflow we knwo them so it should be no problem
| @@ -156,7 +156,7 @@ | |||
| res.insert(0, 'dna_count',(res.dna_sum+1) / (res.n_obs_bc+1) / dna_total * 10**6) | |||
| res.insert(1, 'rna_count',(res.rna_sum+1) / (res.n_obs_bc+1) / rna_total * 10**6) | |||
|
|
|||
| print(res_t) | |||
| #print(res_t) | |||
The option to test specific variants is documented in the Nature Protocols paper but is not implemented here. This PR contains a draft implementation of the variant detection workflow. More importantly, it fixes two crashes, one in each of the association and count workflows.