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

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

Closed
wants to merge 2 commits into from

Conversation

Acribbs
Copy link
Contributor

@Acribbs Acribbs commented Jul 30, 2018

…as incorrectly written

@sebastian-luna-valero
Copy link
Member

Hi,

This reverts the changes in #21 .

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

Best regards,
Sebastian

@Acribbs
Copy link
Contributor Author

Acribbs commented Aug 21, 2018

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
Copy link
Member

Thanks. For our reference, this was also removed by @jscaber in #43

@nickilott please let us know your thoughts.

@Acribbs
Copy link
Contributor Author

Acribbs commented Aug 21, 2018

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

@nickilott
Copy link
Contributor

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
Copy link
Contributor Author

Acribbs commented Aug 22, 2018

I also agree

@sebastian-luna-valero
Copy link
Member

Thanks, both.

Closing this in favor of #43

@sebastian-luna-valero sebastian-luna-valero deleted the AC-update_merge branch August 22, 2018 14:26
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

Successfully merging this pull request may close these issues.

3 participants