Skip to content

Commit

Permalink
Added a test, but realized that Sarek has yet another edge-case. Clea…
Browse files Browse the repository at this point in the history
…r improvement over previous performance, but still...
  • Loading branch information
MatthiasZepper committed Jun 29, 2023
1 parent ccbb131 commit fd3f641
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- Refactored the CLI for `--singularity-cache` in `nf-core download` from a flag to an argument. The prior options were renamed to `amend` (container images are only saved in the `$NXF_SINGULARITY_CACHEDIR`) and `copy` (a copy of the image is saved with the download). `remote` was newly introduced and allows to provide a table of contents of a remote cache via an additional argument `--singularity-cache-index` ([#2247](https://github.com/nf-core/tools/pull/2247)).
- Refactored the CLI parameters related to container images. Although downloading other images than those of the Singularity/Apptainer container system is not supported for the time being, a generic name for the parameters seemed preferable. So the new parameter `--singularity-cache-index` introduced in [#2247](https://github.com/nf-core/tools/pull/2247) has been renamed to `--container-cache-index` prior to release ([#2336](https://github.com/nf-core/tools/pull/2336)).
- To address issue [#2311](https://github.com/nf-core/tools/issues/2311), a new parameter `--container-library` was created allowing to specify the container library (registry) from which container images in OCI format (Docker) should be pulled ([#2336](https://github.com/nf-core/tools/pull/2336)).
- Container detection in configs was improved. This allows for DSL2-like container definitions inside the container parameter value provided to process scopes [#2346](https://github.com/nf-core/tools/pull/2346).

#### Updated CLI parameters

Expand Down
4 changes: 2 additions & 2 deletions nf_core/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ def find_container_images(self, workflow_directory):

# Find any config variables that look like a container
for k, v in self.nf_config.items():
if k.startswith("process.") and k.endswith(".container"):
if (k.startswith("process.") or k.startswith("params.")) and k.endswith(".container"):
"""
Can be plain string / Docker URI or DSL2 syntax
Expand Down Expand Up @@ -910,7 +910,7 @@ def rectify_raw_container_matches(self, raw_findings):
cleaned_matches.append(this_container)
else:
log.error(
f"[red]Cannot parse container string in '{file_path}':\n\n{textwrap.indent(capture, ' ')}\n\n:warning: Skipping this singularity image."
f"[red]Cannot parse container string in '{file_path}':\n\n{textwrap.indent(container_value, ' ')}\n\n:warning: Skipping this singularity image."
)

return cleaned_matches
Expand Down
38 changes: 36 additions & 2 deletions tests/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import nf_core.utils
from nf_core.download import ContainerError, DownloadWorkflow, WorkflowRepo
from nf_core.synced_repo import SyncedRepo
from nf_core.utils import NFCORE_CACHE_DIR, NFCORE_DIR
from nf_core.utils import NFCORE_CACHE_DIR, NFCORE_DIR, nextflow_cmd

from .utils import with_temporary_file, with_temporary_folder

Expand Down Expand Up @@ -135,7 +135,7 @@ def test_wf_use_local_configs(self, tmp_path):
#
@with_temporary_folder
@mock.patch("nf_core.utils.fetch_wf_config")
def test_find_container_images(self, tmp_path, mock_fetch_wf_config):
def test_find_container_images_config_basic(self, tmp_path, mock_fetch_wf_config):
download_obj = DownloadWorkflow(pipeline="dummy", outdir=tmp_path)
mock_fetch_wf_config.return_value = {
"process.mapping.container": "cutting-edge-container",
Expand All @@ -145,6 +145,40 @@ def test_find_container_images(self, tmp_path, mock_fetch_wf_config):
assert len(download_obj.containers) == 1
assert download_obj.containers[0] == "cutting-edge-container"

#
# Test for 'find_container_images' in config with nextflow
#
@pytest.mark.skipif(
shutil.which("nextflow") is None,
reason="Can't run test that requires nextflow to run if not installed.",
)
@with_temporary_folder
@mock.patch("nf_core.utils.fetch_wf_config")
def test__find_container_images_config_nextflow(self, tmp_path, mock_fetch_wf_config):
download_obj = DownloadWorkflow(pipeline="dummy", outdir=tmp_path)
nfconfig_raw = nextflow_cmd(
f"nextflow config -flat {Path(__file__).resolve().parent / 'data/mock_config_containers'}"
)
config = {}
for l in nfconfig_raw.splitlines():
ul = l.decode("utf-8")
try:
k, v = ul.split(" = ", 1)
config[k] = v.strip("'\"")
except ValueError:
pass
mock_fetch_wf_config.return_value = config
download_obj.find_container_images("workflow")
assert len(download_obj.containers) == 4
assert "nfcore/methylseq:1.0" in download_obj.containers
assert "nfcore/methylseq:1.4" in download_obj.containers
assert "nfcore/sarek:dev" in download_obj.containers
assert "https://depot.galaxyproject.org/singularity/r-shinyngs:1.7.1--r42hdfd78af_1" in download_obj.containers
# does not yet pick up nfcore/sarekvep:dev.${params.genome}, because successfully detecting "nfcore/sarek:dev"
# breaks the loop already. However, this loop-breaking is needed to stop iterating over DSL2 syntax if a
# direct download link has been found. Unless we employ a better deduplication, support for this kind of
# free-style if-else switches will sadly remain insufficient.

#
# Tests for 'singularity_pull_image'
#
Expand Down

0 comments on commit fd3f641

Please sign in to comment.