-
Notifications
You must be signed in to change notification settings - Fork 142
Release: 5.0.0 Green Squirrel #864
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
Co-authored-by: James A. Fellows Yates <[email protected]>
Co-authored-by: James A. Fellows Yates <[email protected]>
Co-authored-by: James A. Fellows Yates <[email protected]>
nf-test: default config
Skip short and long read qc
Fix docs typo
Co-authored-by: Simon Pearce <[email protected]>
Co-authored-by: Simon Pearce <[email protected]>
New config test_single_end
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 didn't detect any blocking issue, only minor suggestions (and typos).
…d by the tool" This reverts commit 69a955e.
conf/modules.config
Outdated
| "--min_af ${params.gtdbtk_min_af}", | ||
| "--pplacer_cpus ${params.gtdbtk_pplacer_cpus}", | ||
| params.gtdbtk_use_full_tree ? "-f" : "", | ||
| "--skip_ani_screen", |
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 would not put this in the general modules.config file, as this disables ANI screening for fast genome classification using skani (I think the fact it was set if the mash db wasn't provided was actually a bug with the old version of the module).
Maybe better to make a param for it (default false) and set it to true in the tests?
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.
Ahh ok
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.
So --run_skani_ani?
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.
Or other way round skip and we set to true in our config?
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.
We have params.gtdbtk_use_full_tree, how about params.gtdbtk_skip_ani_screen?
Set to false by default, but true in test.config?
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 I think I've implemented what you suggested
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.
Let me know If I did that properly
Co-authored-by: Jim Downie <[email protected]>
|
@nf-core-bot fix linting |
erikrikarddaniel
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.
👍
…dmundmiller hack to get SPADES to work on Fusion
…re on exit code 1 for MAG_DEPTHS_PLOTS,
…ue to summarise problem
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).