orthofinder: remove mv line

[FernandoDuarteF]

Changes

Removed mv line from orthofinder. This line is not necessary, and pipelines using this module might break during the AWS megatests.

Comments

Only nf-core/genomeqc uses this module.

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 module conventions in the contribution docs
• If necessary, include test data in your PR.
• Remove all TODO statements.
• Broadcast software version numbers to topic: versions - See version_topics
• Follow the naming conventions.
• Follow the parameters requirements.
• Follow the input/output options guidelines.
• Add a resource label
• 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:
  • For modules:
    • nf-core modules test <MODULE> --profile docker
    • nf-core modules test <MODULE> --profile singularity
    • nf-core modules test <MODULE> --profile conda
  • For subworkflows:
    • nf-core subworkflows test <SUBWORKFLOW> --profile docker
    • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
    • nf-core subworkflows test <SUBWORKFLOW> --profile conda

[SPPearce]

This is functional, because it means that the output changes. I don't think that mv is an issue on AWS?

[FernandoDuarteF]

This is functional, because it means that the output changes. I don't think that mv is an issue on AWS?

I think the line I added here:
results_dir = prior_run ? "${prior_run}/OrthoFinder/Results_${prefix}" : "input/OrthoFinder/Results_${prefix}"
should be able to point to the two different outputs without the need for renaming the output directory.

It can fail on AWS (it happened on nf-core/genomeqc) because S3's mv does not rename the folder, instead it does a full copy + delete operation. Orthofinder's output folder can have +1000 files. The probability that it'll break is pretty high.

[chriswyatt1]

I think its better with your changes, if we don't have to move the output files at all.
There can be thousands of files, and each will have to change location on S3 in the way we had it.

[SPPearce]

Hmm, but the file operation should only be on the local machine, before anything is copied to s3, rather than afterwards.

[chriswyatt1]

Oh, I didn't realise that. That makes sense, not sure why it was failing then

[SPPearce]

Currently the mv command is meaning that the output of the module is just $prefix, rather than a nested output.
Is there no way to convince the tool to put its output files in the current directory, rather than into a subdirectory in the input folder (which seems a strange place for it to go)

[FernandoDuarteF]

Hmm, but the file operation should only be on the local machine, before anything is copied to s3, rather than afterwards.

Honestly I'm not familiar with how AWS megatests work (or S3, for the matter). But I think what I said above is what's happening here? https://cloud.seqera.io/orgs/nf-core/workspaces/AWSmegatests/watch/4JIMRp0ObLAkso/logs

Seems to me the mv line is the issue.

I was also told that mv was not recommended because of the whole copy-delete thing on S3.

[FernandoDuarteF]

Currently the mv command is meaning that the output of the module is just $prefix, rather than a nested output. Is there no way to convince the tool to put its output files in the current directory, rather than into a subdirectory in the input folder (which seems a strange place for it to go)

Hmm I can have a look. But maybe @chriswyatt1 has an answer?