-
Notifications
You must be signed in to change notification settings - Fork 142
Longread only functionality #718
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
Conversation
… 97 to 90 when dealing with longreads
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 3.2.1. For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation. |
d4straub
left a comment
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.
I only had a short look, sorry, but I just wanted to express my gratitude that you tackle long read assembly!
My pleasure hehe. Still WIP. I have to tidy up the code and run some validation on real data, but we're getting there! |
…rtread assemblers, when assembly input is given
…empty if no files are given
…d also return remainder in case the ch_short_reads channel is empty. Change config for FILTLONG to only use '--trim' option if shortreads are passed
|
@jfy133 thanks for the review. I think I have addressed your suggested changes. I hope the tests go well! |
…updated nf-core module for samtools/faidx
|
FYI all - we have had a pretty catastrophic outage on our cluster and I'm not currently able to finish the test I was running - and it's unlikely I'll get a chance to finish it any time soon. But I'm happy with the PR as-is - the long-read bits were working well after we fixed those wee bugs! |
|
Thanks for the heads up Jim! @muabnezor I will check the changes later today or tomorrow. But our lab have just started getting our own nanopore data, so I'll test in that myself. It's only isolate data so shouldn't take too long |
|
@nf-core-bot fix linting |
|
@muabnezor From my test - and now looking at Is that to be expected? This is one of the critical output files... |
jfy133
left a comment
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.
Code-wise I'm happy now!
I just want to explore the results a bit more (see comment above about missing bin_summary.tsv process
| longread_percentidentity = null | ||
| shortread_percentidentity = null |
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.
@muabnezor is there any reason why you didn't set this?
no, this is not expected, are any of the channels from your test empty going into the process? |
I'm running another long running test on my laptop at the moment so I can't check (unfortuantely)... any chance you could look? Otherwise I will do tomorrow |
Sorry, can't get to that this week, I will have a look at it on monday |
|
@jfy133 |
|
I did some exploration last night: https://nfcore.slack.com/archives/CE9MS66BS/p1749154196789889?thread_ts=1748503770.520549&cid=CE9MS66BS Essentially for some reason there is an extra meta being added somewhere meaning the reeads and bins can't be @dialvarezs as you are waiting for this to be able to continue with your stuff, any chance you could have a look? |
|
And to clarify @muabnezor it's not the DEPTHS subworkflow itself that is the issue, just the contents of the input channels :) |
|
@jfy133, there, I think I fixed it in the last commit in how depth channel is returned from binning workflow. I think (and hope!) this will fix it. Thanks for helping with this! |
|
@muabnezor testing! |
jfy133
left a comment
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.
Can confirm bin_summary.tsv now generated!
Everything else we can fix in post-production 😬
Thank you very very very much @muabnezor ! For the huge effort and your patience on this one, like I said before - many people are going to be very happy!
Hit that merge button!
|
@jfy133 just merged! Thanks for all the help with this PR! |
This PR adds long-read only functionality to mag.
closes #662, #659, #275
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).