modules icon indicating copy to clipboard operation
modules copied to clipboard

add eggnog-mapper module

Open rpetit3 opened this issue 2 years ago • 2 comments

PR checklist

Closes https://github.com/nf-core/modules/issues/905

  • [x] This comment contains a description of changes (with reason).
  • [x] If you've fixed a bug or added code that should be tested, add tests!
  • [x] If you've added a new tool - have you followed the module conventions in the contribution docs
  • [x] If necessary, include test data in your PR.
  • [x] Remove all TODO statements.
  • [x] Emit the versions.yml file.
  • [x] Follow the naming conventions.
  • [x] Follow the parameters requirements.
  • [x] Follow the input/output options guidelines.
  • [x] Add a resource label
  • [x] Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • [x] PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd
    • [x] PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd
    • [x] PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd

I had to use -stub-run because eggNOG uses 10+GB databases files as inputs. I will be testing locally to make sure this works with a real run.

rpetit3 avatar Nov 03 '21 21:11 rpetit3

I'd like to use this module, and as far as I can tell the failing tests are due to the need to not download the huge db's, is that correct @rpetit3? Did you get to the bottom with if this is an okay practice? If not, I stumbled across this but I'm not really sure it's compatible with this module. Is there anything else that need fixing before this can be merged (probably updating the DSL2-syntax a bit), or is it a matter of an approving review?

emnilsson avatar Feb 01 '22 14:02 emnilsson

Hi @emnilsson

Slightly modified but, I'm using this in Bactopia which is doing tests on actual database.

Tests: https://github.com/bactopia/bactopia/blob/master/subworkflows/local/eggnog/test.yml Module: https://github.com/bactopia/bactopia/blob/master/modules/nf-core/modules/eggnog/mapper/main.nf

You are right though, it will have to be updated to the current syntax. I might be able to get around to that later in the week.

Let me know what you think

rpetit3 avatar Feb 01 '22 14:02 rpetit3

Hi @rpetit3, thank you for this PR! We are merging as many modules as possible now due to and impending restructuring of the entire repo. This will mean you will need to update the module to reflect these changes before it can be merged in the future. It appears like this module isn't ready to be merged so if applicable, we are converting it to draft and adding the WIP label. If this isn't the case please let us know and we will try to get the module in before the changes. Thanks again!

JoseEspinosa avatar Sep 28 '22 09:09 JoseEspinosa

Oh, wow, this one still has the functions.nf. I think one all the changes are in place, I might just restart from a fresh modules create

rpetit3 avatar Sep 28 '22 14:09 rpetit3

Then I will close this PR, feel free to reopen it in case you think otherwise.

JoseEspinosa avatar Sep 28 '22 15:09 JoseEspinosa