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 MALT metagenomic classifier functionality #284

Closed
wants to merge 33 commits into from
Closed

Add MALT metagenomic classifier functionality #284

wants to merge 33 commits into from

Conversation

jfy133
Copy link
Member

@jfy133 jfy133 commented Nov 22, 2019

This begins adding metagenomic classifier functionality to EAGER primarily for pathogen screeening.

The exact classifer can be specified - but currently only MALT is implemented (as precursor for HOPS).

Path in github actions test for database downloading will be pointed to nf-core once nf-core/test-datasets#97

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 necessary, also make a PR on the nf-core/eager branch on the nf-core/test-datasets repo
  • Ensure the test suite passes (nextflow run . -profile test,docker).
  • Make sure your code lints (nf-core lint .).
  • Documentation in docs is updated
  • CHANGELOG.md is updated
  • README.md is updated

Learn more about contributing: https://github.com/nf-core/eager/tree/master/.github/CONTRIBUTING.md

@jfy133 jfy133 added the ready label Nov 22, 2019
@jfy133 jfy133 added this to the V2.1 "Ulm" milestone Nov 22, 2019
@apeltzer
Copy link
Member

Can you update the tests as the PR you mentioned has been merged now? That will then perform a final tests upon which I'm reviewing then - thanks a bunch!

@jfy133
Copy link
Member Author

jfy133 commented Nov 23, 2019

Can you update the tests as the PR you mentioned has been merged now? That will then perform a final tests upon which I'm reviewing then - thanks a bunch!

Done! I've only added github actions testing though. Let me know if you also want travis a the same time and I'll work out how to add it (it's a little complicated because I have to first download the MALT databsae, as MALT expects a dir rather than files)

Copy link
Member

@apeltzer apeltzer left a comment

Choose a reason for hiding this comment

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

Needs some more documentation please!

@jfy133 jfy133 added WIP Work in progress and removed ready labels Nov 25, 2019
@jfy133
Copy link
Member Author

jfy133 commented Nov 25, 2019

Needs some more documentation please!

Oops sorry - I thought MALT was merged already! So I just started using the same branch for the next step. The new commit isn't actually working yet ;).

Reflagged as WIP!

@jfy133 jfy133 self-assigned this Nov 25, 2019
@jfy133
Copy link
Member Author

jfy133 commented Nov 25, 2019

@apeltzer assuming tests go through, this should be ready.

Note, to compensate for the lack of documentation in your last comment - I added lots extra for you to check ;).

@jfy133 jfy133 requested a review from apeltzer November 25, 2019 21:19
@jfy133 jfy133 added ready and removed WIP Work in progress labels Nov 25, 2019
@jfy133 jfy133 mentioned this pull request Nov 25, 2019
@jfy133 jfy133 added WIP Work in progress and removed ready labels Nov 26, 2019
@jfy133
Copy link
Member Author

jfy133 commented Nov 26, 2019

I'm closing this as Im finding problems and it doesn't make sense to run github actions on my personal account and here. Will make a new one onceit's working!

@jfy133 jfy133 closed this Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants