Fix cellranger multi handling of crispr data

[svigneau]

PR checklist

Closes #7174

• This comment contains a description of changes (with reason).
• 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

The proposed changes are described in issue 7174 and intended to fix a bug that prevents handling of CRISPR data when using cellranger multi with gene expression and CRISPR libraries.

The addition of a test dataset with CRISPR data has been requested in PR 1415. A dedicated test for CRISPR data handling by cellranger multi will be submitted in a separate PR once the test dataset is approved.

I removed testing using conda profile from the checklist because running it leads to the following error related to a different module: CELLRANGER_MKGTF module does not support Conda. Please use Docker / Singularity / Podman instead.

[famosab]

We need to add this tool to .github/conda_skip.yml then. Can you do that?

[fmalmeida]

Hi @svigneau ,
Thanks for the code work here 😄
There are few things before we can consider it done.

1. We have to skip conda checks as highlighted by @famosab here
2. Now that the test-datasets for crispr are available, we must also include the nf-test run for crispr. Can you add that?

Other than that, it is good.

[svigneau]

Thanks @famosab and @fmalmeida !

I have added the new tests for running cellranger multi with GEX + CRISPR data in modules/nf-core/cellranger/multi/tests/main.nf.test. Please, let me know what you think!

Following the existing pattern, I also updated the modules/tests/config/test_data.config file. However, pre-existing and new cellranger multi tests do not use the aliases specified in this file. Do you know what this file is used for?

As for skipping conda tests, modules/nf-core/cellranger/multi was already in .github/conda_skip.yml, but this didn't prevent the conda tests from running. This could be related to the issue mentioned here. Is there an alternate way to prevent being blocked by conda tests failing, or is it better to wait until the mentioned issue gets fixed before finalizing the merge request?

[fmalmeida]

Hi @svigneau ,
Thanks for the updates.

1. It looks good. Very much aligned with the syntax of the other tests so ... awesome 😄. Question here, did you check the working directory of your test to verify if the executed commands, cellranger multi config files and results have been produced as expected? If so, then we are good here.
2. This file test_data.config is used to load the file paths as parameters, so you could for instance use: params.test_data.sarscov2.genome.genome_fasta.
3. If the conda_skip is being ignored, there is not much we can do. We can ask a nf-core maintainer to know what to do. If, for example, we can just ignore the conda fails given the container tests are passing.

[svigneau]

Thanks, @fmalmeida !

Following up on each point:

1. I did check that the expected files have been created with sensible content in the .nf-test/tests directory.
2. Thanks for clarifying, and good to know for the future!
3. I'll follow up on Slack with an nf-core maintainer about fixing the handling of .github/conda_skip.yml.

[famosab]

I think the conda_skip issue nreeds to be fixed otherwise we cannot merge because of the branch protection rules :)

[maxulysse]

I think the conda_skip issue nreeds to be fixed otherwise we cannot merge because of the branch protection rules :)

An admin can force merge 😎

[svigneau]

As an update, @mashehu is working on making conda skip work again (Slack thread). So I think it makes sense to wait for the fix.