Skip to content
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

Add kmcp classifier #312

Merged
merged 77 commits into from
Aug 19, 2023
Merged

Add kmcp classifier #312

merged 77 commits into from
Aug 19, 2023

Conversation

sofstam
Copy link
Collaborator

@sofstam sofstam commented Jun 27, 2023

This PR closes #210

PR checklist

  • This 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/taxprofiler 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.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@github-actions
Copy link

github-actions bot commented Jun 27, 2023

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit e5df2f9

+| ✅ 157 tests passed       |+
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline

✅ Tests passed:

Run details

  • nf-core/tools version 2.9
  • Run at 2023-08-19 08:44:30

@sofstam sofstam added the WIP Work in progress label Jun 27, 2023
@sofstam sofstam changed the title Start adding kmcp Add kmcp classifier Jun 27, 2023
@sofstam
Copy link
Collaborator Author

sofstam commented Jul 20, 2023

@jfy133 I have added the KMCP_COMPUTE in the profiling workflow. I get an error for KMCP_INDEX:
intput and output paths should not be the same

In the module test, this was adressed with:

https://github.com/nf-core/modules/blob/master/tests/modules/nf-core/kmcp/index/nextflow.config

I am not sure how this should be implemented here. Shall I create a new def new_meta ?

@sofstam sofstam marked this pull request as ready for review August 1, 2023 11:46
sofstam and others added 9 commits August 15, 2023 15:36
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]>
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]>
Co-authored-by: James A. Fellows Yates <[email protected]>
Co-authored-by: James A. Fellows Yates <[email protected]>
@jfy133
Copy link
Member

jfy133 commented Aug 15, 2023

You've installed KMCP merge but it's not used anywhere, is that correct? I would be expecting this in standardisation_profiles.nf and modules.conf if so.

I will update the metromap diagram next review :)

I forgot to remove the module, updating the PR now.

What does merge do exactly though? Multiple samples profiles in one? That would be useful!

It is merge results from different databases.

Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may and try implement the remaining changes myself now if I have time

CHANGELOG.md Outdated Show resolved Hide resolved
docs/output.md Outdated Show resolved Hide resolved
docs/output.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
subworkflows/local/profiling.nf Outdated Show resolved Hide resolved
subworkflows/local/profiling.nf Outdated Show resolved Hide resolved
subworkflows/local/profiling.nf Outdated Show resolved Hide resolved
subworkflows/local/profiling.nf Outdated Show resolved Hide resolved
subworkflows/local/profiling.nf Outdated Show resolved Hide resolved
@jfy133 jfy133 self-requested a review August 19, 2023 09:22
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the patience @sofstam ! That was a complicated one!

@jfy133 jfy133 merged commit 697647f into nf-core:dev Aug 19, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement for existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants