-
Notifications
You must be signed in to change notification settings - Fork 110
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
Fix MaxBin2 abundance input #690
Conversation
I would rather these to go in the official module @d4straub but I'm around for fast review for that too |
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.
Looking good so far, just I would request porting this to the official module (like I said above, I'm available for rapid review on that)
My last question is (being naive but want to make sure this is recorded for future me):
I was just thinking @uel3 mentioned this issue in the context of co-assembly, do we need to distinguish the mechanism being added here between standalone or co-assembly?
Reading the minimal Maxbin2 docs I think it doesn't really matter - so we can always do it in this way, but I just want to double check as you likely have more experience than I have with the tool.
Thasnks, PR to modules is open.
Not sure I understand the question. In my tests this works with and without co-assembly, it depends rather on |
OK will look now!
That answers my question basically 👍 |
@d4straub module can be installed in this PR now :) |
Just going to run a test with current version and your changes to compare, but code looks good 👍 |
@d4straub I had a look and it seems to run through, however when I run with |
@jfy133 I checked the code and The following works as expected for me (this runs on this modified branch), i.e.
so I also would not expect any results from binners. Edit: clarity |
OK that's strange, maybe I just had a weird test configuration (using the |
@d4straub I will merge in and then do the template merge |
Fix #663
I adopted for
CONVERT_DEPTHS
largely the solution by @uel3 but changes theMaxBin2
module so that it accepts depth files via-abund
, I think this is a more elegant solution. Additionally theCONVERT_DEPTHS
module was changed so that the abundance files are named by samples, that makes the*.abundance
files by MaxBin2 include the sample names, i.e. they are meaningful now.The nf-core module maxbin2 was updated separately and is used here.
Linting with nf-core tools version 2.14.1 succeeds.
PR checklist
nf-core lint
).nextflow run . -profile test,docker --outdir <OUTDIR>
).nextflow run . -profile debug,test,docker --outdir <OUTDIR>
).docs/usage.md
is updated.docs/output.md
is updated.CHANGELOG.md
is updated.README.md
is updated (including new tool citations and authors/contributors).