search

cgat-developers / cgat-flow

cgat-flow repository
MIT License
13 stars 9 forks source link

have updated regex of ruffus command for mergeReadCounts because it w… #45

Closed Acribbs closed 5 years ago

Acribbs commented 5 years ago

…as incorrectly written

sebastian-luna-valero commented 5 years ago

Hi,

This reverts the changes in https://github.com/cgat-developers/cgat-flow/pull/21 .

Could @Acribbs and @nickilott please have a look and decide what's the best way forward?

Best regards, Sebastian

Acribbs commented 5 years ago

mergeBAMFiles (and hence mergeReadCounts) is no longer in the full task because it breaks the pipeline testing (if I remember rightly), therefore this function is not tested at the moment. So if the mergeReadCounts is activated it results in a nreads.dir/nreads.dir/{sample_name}.nreads, it should be nreads.dir/{sample_name}.nreads. The pipeline won't run mergeBAMFiles because the regular expression is wrong.

I have ran mapping using the committed code and it works now.

sebastian-luna-valero commented 5 years ago

Thanks. For our reference, this was also removed by @jscaber in https://github.com/cgat-developers/cgat-flow/pull/43

@nickilott please let us know your thoughts.

Acribbs commented 5 years ago

Ah yes, this is exactly the issue I am correcting for here, may be better to merge @jscaber 's pull request then.

nickilott commented 5 years ago

Hi, Yes the nreads.dir/nreads.dir should be changed as in #43 and above. The issue that I was having a problem with was loading the read counts (duplication) but this (as far as I can tell) is not changed in #43. I think it is fine to merge @jscaber pull request if everyone else is in agreement.

Apologies again for not responding sooner to this

Nick

Acribbs commented 5 years ago

I also agree

sebastian-luna-valero commented 5 years ago

Thanks, both.

Closing this in favor of https://github.com/cgat-developers/cgat-flow/pull/43