Skip to content

Commit

Permalink
Removes all remaining references to max_* params
Browse files Browse the repository at this point in the history
  • Loading branch information
jfy133 committed Sep 4, 2024
1 parent 2e690d9 commit 736d5e8
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 109 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/create-test-wf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ jobs:
pwd
# echo content of current directory
ls -la
nextflow run nf-core-testpipeline -profile test,self_hosted_runner --outdir ./results --max_cpus 4
nextflow run nf-core-testpipeline -profile test,self_hosted_runner --outdir ./results
- name: Upload log file artifact
if: ${{ always() }}
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
- add option to exclude test configs from pipeline template ([#3133](https://github.com/nf-core/tools/pull/3133))
- add option to exclude tower.yml from pipeline template ([#3134](https://github.com/nf-core/tools/pull/3134))
- Add tests to ensure all files are part of a template customisation group and all groups are tested ([#3099](https://github.com/nf-core/tools/pull/3099))
- Replaces the old custom `check_max()` function with the Nextflow native `resourceLimits` directive ([#3037](https://github.com/nf-core/tools/pull/3037))

### Linting

Expand Down Expand Up @@ -98,7 +99,6 @@

- Don't cache pip in `linting.yml` ([#2961](https://github.com/nf-core/tools/pull/2961))
- Lint pipelines with the nf-core template version and post comment if it is outdated ([#2978](https://github.com/nf-core/tools/pull/2978))
- Replaces the old custom `check_max()` function with the Nextflow native `resourceLimits` directive ([#3037](https://github.com/nf-core/tools/pull/3037))

### General

Expand Down
16 changes: 13 additions & 3 deletions nf_core/pipelines/lint/nextflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def nextflow_config(self) -> Dict[str, List[str]]:
* ``params.nf_required_version``: The old method for specifying the minimum Nextflow version. Replaced by ``manifest.nextflowVersion``
* ``params.container``: The old method for specifying the dockerhub container address. Replaced by ``process.container``
* ``igenomesIgnore``: Changed to ``igenomes_ignore``
* ``params.max_cpus``: Old method of specifying the maximum number of CPUs a process can request. Replaced by native Nextflow `resourceLimits`directive.
* ``params.max_cpus``: Old method of specifying the maximum number of CPUs a process can request. Replaced by native Nextflow `resourceLimits`directive in config files.
* ``params.max_memory``: Old method of specifying the maximum number of memory can request. Replaced by native Nextflow `resourceLimits`directive.
* ``params.max_time``: Old method of specifying the maximum number of CPUs can request. Replaced by native Nextflow `resourceLimits`directive.
Expand Down Expand Up @@ -149,7 +149,13 @@ def nextflow_config(self) -> Dict[str, List[str]]:
["params.input"],
]
# Throw a warning if these are missing
config_warn = [["manifest.mainScript"], ["timeline.file"], ["trace.file"], ["report.file"], ["dag.file"]]
config_warn = [
["manifest.mainScript"],
["timeline.file"],
["trace.file"],
["report.file"],
["dag.file"],
]
# Old depreciated vars - fail if present
config_fail_ifdefined = [
"params.nf_required_version",
Expand Down Expand Up @@ -207,7 +213,11 @@ def nextflow_config(self) -> Dict[str, List[str]]:
)

# Remove field that should be ignored according to the linting config
ignore_configs = self.lint_config.get("nextflow_config", []) if self.lint_config is not None else []
ignore_configs = (
self.lint_config.get("nextflow_config", [])
if self.lint_config is not None
else []
)

for cfs in config_fail:
for cf in cfs:
Expand Down
87 changes: 9 additions & 78 deletions tests/pipelines/lint/test_nextflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ def test_default_values_match(self):
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 0
assert len(result["warned"]) == 0
assert "Config default value correct: params.max_cpus" in str(result["passed"])
assert "Config default value correct: params.validate_params" in str(result["passed"])
assert "Config default value correct: params.validate_params" in str(
result["passed"]
)

def test_nextflow_config_bad_name_fail(self):
"""Tests that config variable existence test fails with bad pipeline name"""
Expand Down Expand Up @@ -69,88 +70,16 @@ def test_nextflow_config_missing_test_profile_failed(self):
assert len(result["failed"]) > 0
assert len(result["warned"]) == 0

def test_default_values_fail(self):
"""Test linting fails if the default values in nextflow.config do not match the ones defined in the nextflow_schema.json."""
# Change the default value of max_cpus in nextflow.config
nf_conf_file = Path(self.new_pipeline) / "nextflow.config"
with open(nf_conf_file) as f:
content = f.read()
fail_content = re.sub(r"\bmax_cpus\s*=\s*16\b", "max_cpus = 0", content)
with open(nf_conf_file, "w") as f:
f.write(fail_content)
# Change the default value of max_memory in nextflow_schema.json
nf_schema_file = Path(self.new_pipeline) / "nextflow_schema.json"
with open(nf_schema_file) as f:
content = f.read()
fail_content = re.sub(r'"default": "128.GB"', '"default": "18.GB"', content)
with open(nf_schema_file, "w") as f:
f.write(fail_content)
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 2
assert (
"Config default value incorrect: `params.max_cpus` is set as `16` in `nextflow_schema.json` but is `0` in `nextflow.config`."
in result["failed"]
)
assert (
"Config default value incorrect: `params.max_memory` is set as `18.GB` in `nextflow_schema.json` but is `128.GB` in `nextflow.config`."
in result["failed"]
)

def test_catch_params_assignment_in_main_nf(self):
"""Test linting fails if main.nf contains an assignment to a parameter from nextflow_schema.json."""
# Add parameter assignment in main.nf
main_nf_file = Path(self.new_pipeline) / "main.nf"
with open(main_nf_file, "a") as f:
f.write("params.max_time = 42")
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 1
assert (
result["failed"][0]
== "Config default value incorrect: `params.max_time` is set as `240.h` in `nextflow_schema.json` but is `null` in `nextflow.config`."
)

def test_allow_params_reference_in_main_nf(self):
"""Test linting allows for references like `params.aligner == 'bwa'` in main.nf. The test will detect if the bug mentioned in GitHub-issue #2833 reemerges."""
# Add parameter reference in main.nf
main_nf_file = Path(self.new_pipeline) / "main.nf"
with open(main_nf_file, "a") as f:
f.write("params.max_time == 42")
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 0

def test_default_values_ignored(self):
"""Test ignoring linting of default values."""
# Add max_cpus to the ignore list
nf_core_yml_path = Path(self.new_pipeline) / ".nf-core.yml"
nf_core_yml = NFCoreYamlConfig(
repository_type="pipeline", lint={"nextflow_config": [{"config_defaults": ["params.max_cpus"]}]}
)
with open(nf_core_yml_path, "w") as f:
yaml.dump(nf_core_yml.model_dump(), f)

lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
lint_obj._load_lint_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 0
assert len(result["ignored"]) == 1
assert "Config default value correct: params.max_cpu" not in str(result["passed"])
assert "Config default ignored: params.max_cpus" in str(result["ignored"])

def test_default_values_float(self):
"""Test comparing two float values."""
# Add a float value `dummy=0.0001` to the nextflow.config below `validate_params`
nf_conf_file = Path(self.new_pipeline) / "nextflow.config"
with open(nf_conf_file) as f:
content = f.read()
fail_content = re.sub(
r"validate_params\s*=\s*true", "params.validate_params = true\ndummy = 0.000000001", content
r"validate_params\s*=\s*true",
"params.validate_params = true\ndummy = 0.000000001",
content,
)
with open(nf_conf_file, "w") as f:
f.write(fail_content)
Expand Down Expand Up @@ -180,7 +109,9 @@ def test_default_values_float_fail(self):
with open(nf_conf_file) as f:
content = f.read()
fail_content = re.sub(
r"validate_params\s*=\s*true", "params.validate_params = true\ndummy = 0.000000001", content
r"validate_params\s*=\s*true",
"params.validate_params = true\ndummy = 0.000000001",
content,
)
with open(nf_conf_file, "w") as f:
f.write(fail_content)
Expand Down
Loading

0 comments on commit 736d5e8

Please sign in to comment.