-
Notifications
You must be signed in to change notification settings - Fork 6
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
Assemble ONT genomes #516
Assemble ONT genomes #516
Conversation
96b1d4a
to
b60d62e
Compare
b60d62e
to
22dbf56
Compare
22dbf56
to
0f9c23d
Compare
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.
Nice work! I just have one question 😄
deepvariant_model_type = params.preset == 'ONT_R10' ? 'ONT_R104' : 'PACBIO' | ||
hifiasm_preset = params.preset == 'ONT_R10' ? '--ont' : '' | ||
minimap2_read_mapping_preset = params.preset == 'ONT_R10' ? 'lr:hq' : 'map-hifi' |
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 am not sure I follow this part, if params.preset == 'ONT_R10'
then hifiasm_preset = --ont
, otherwise its empty, right? If so, won't whatever is given as a parameter --hifiasm_preset
be overwritten?
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.
Yes, then it will be empty. However, it should be working the other way around, so whatever is given with --hifiasm_preset
on the command line will overwrite what is set in nextflow.config
(''
or --ont
).
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.
ok, that makes sense then 😄
Perhaps it would be good to add a test?
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.
A test with --preset ONT_R10
is already there if that's what you mean, you can see the snapshot of the assembly outputs in tests/samplesheet_multisample_ont_bam.nf.test.snap
.
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.
Sorry, yeah you are right, I was thinking more on the test config side, but if its there as an nf-core test, that should be enough
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.
Ah, okay. It would be nice, perhaps when I have fixed the stubs so we don't need to do it with a real run.
This parameter is not (edit) very likely to be changed on its own, but I double checked manually and --preset ONT_R10
results in the --ont
flag being added to hifiasm, but --preset ONT_R10 --hifiasm_preset=""
results in nothing added to hifiasm.
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.
🚀
This PR updates hifiasm to 0.24.0, which adds beta support for ONT assembly with flag
--ont
. Therefore assembly is now run by default with theONT_R10
profile.Closes #515 and #518.
PR checklist
nf-core pipelines 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).