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

nf-test for ashlar #6931

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

nf-test for ashlar #6931

wants to merge 4 commits into from

Conversation

kbestak
Copy link
Contributor

@kbestak kbestak commented Nov 4, 2024

Migrate pytest to nf-test for ASHLAR

PR checklist

Closes #6895

  • 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.
  • Emit the versions.yml file.
  • 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 --- CONDA fails tests - not reproducible between Docker - Conda?

@kbestak kbestak requested a review from a team as a code owner November 4, 2024 13:56
modules/nf-core/ashlar/main.nf Outdated Show resolved Hide resolved
@kbestak
Copy link
Contributor Author

kbestak commented Nov 5, 2024

Thanks for the review @SPPearce, I don't fully understand why the conda test is failing as the tool is on bioconda and deployed as a biocontainer. I would assume that's an issue. Should the conda test be skipped?

@SPPearce
Copy link
Contributor

SPPearce commented Nov 5, 2024

There is presumably something inside the test_all.ome.tif file that is different between docker/singularity and conda.
This happens often for some file types, such as bam files, where the header of the file encodes a file path that is different between the two environments (i.e. /opt/bin instead of /usr/bin or something).
Generally at that point we test for existence of the file, or the presence of a string rather than the entire file.

@kbestak
Copy link
Contributor Author

kbestak commented Nov 6, 2024

The issue I'm seeing is that the conda output matched the md5sum in the previous test, even though the environment definition was the same.

@kbestak
Copy link
Contributor Author

kbestak commented Nov 6, 2024

I'm also noticing an issue with application of flat and dark field correction - should be looked into before merging.

@kbestak
Copy link
Contributor Author

kbestak commented Nov 12, 2024

A short update, I've just tested and a newly built Seqera container with ASHLAR from bioconda produces the same outputs as the conda version, but I haven't yet found the precise reason for the difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ASHLAR testing with nf-test
2 participants