Add mmseqs2 taxonomy #343
Merged
Add mmseqs2 taxonomy #343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
jasmezz
reviewed
Feb 29, 2024
|
The PR works locally, however for the test to pass it still requires |
This was
linked to
issues
Mar 12, 2024
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
|
pinging @jasmezz and @jfy133 and @louperelo for review. Pweez :) |
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
jasmezz
requested changes
Mar 19, 2024
jfy133
reviewed
Mar 20, 2024
There was a problem hiding this comment.
I'm with @jasmezz here! Nextflow code/logic looks great, lots of minor structural/pedantic formatting thigns :D very good work @Darcy220606 !
workflows/funcscan.nf
Outdated
Comment on lines
114
to
116
| // The final subworkflow reports need taxonomic classification | ||
| // This can be either on NT or AA level depending on annotation | ||
| // NOTE: (AA tax. classification will be added only when its PR is merged - NOW - only on NT) |
There was a problem hiding this comment.
Maybe say TODO rather than NOTE so easier to identify in tfuture :)
jasmezz
requested changes
Mar 26, 2024
@jasmezz Ive commited and pushed all your changes/suggestions from last week in this latest round, i dont know which version you were reviewing yesterday. Can you check again please ? |
|
@Darcy220606 did you also add my changes already, e.g. the license at the top of the scripts? They are also not displayed for me...? |
jfy133
approved these changes
Mar 29, 2024
There was a problem hiding this comment.
A few minor things, but otherwise I think we are there!! Well done @Darcy220606 !
Do you plan to make an offiical nf-core MMSEQs subworkflow later and replace it at a later date? If you want guidance I have a PR open you can replicate for a different subworkflow and can send you (I can't remember if you've done a nf-core subworkflow or not yet).
…d_mmseqs2_taxonomy
Yes @jfy133 , ill be making a nf-core subworkflow as we discussed. Its in the todo list :) Thanks ill have a look at the PR and use it as a template. |
jasmezz
approved these changes
Apr 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist
This PR adds teh taxonomic classification of contigs in a step prior enterig contig annotations.
This PR includes solving other issues and therefore closes issue Unify output of workflow summary tools #233 Reorder/rename hamronization summary columns #333 Include Contig taxonomic classification before going into the summary modules #318
For this PR to pass, it requires fixing a bug in
mmseqs/createtsvmodule. UPDATE: FINALLY MERGEDThis comment contains a description of changes (with reason).
If you've fixed a bug or added code that should be tested, add tests!
If you've added a new tool - have you followed the pipeline conventions in the contribution docs
If necessary, also make a PR on the nf-core/funcscan branch on the nf-core/test-datasets repository.
Make sure your code lints (
nf-core lint).Ensure the test suite passes (
nextflow run . -profile test,docker --outdir <OUTDIR>).Usage Documentation in
docs/usage.mdis updated.Output Documentation in
docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).