Skip to content

Commit

Permalink
Merge pull request #3228 from mashehu/fix-yml-config-type
Browse files Browse the repository at this point in the history
allow mixed str and dict in lint config
  • Loading branch information
mashehu authored Dec 3, 2024
2 parents 9d3d074 + eed0598 commit a7351a2
Show file tree
Hide file tree
Showing 16 changed files with 413 additions and 114 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

### Linting

- General: Run pre-commit when testing linting the template pipeline ([#3280](https://github.com/nf-core/tools/pull/3280))
- allow mixed `str` and `dict` entries in lint config ([#3228](https://github.com/nf-core/tools/pull/3228))

### Modules

Expand Down Expand Up @@ -55,6 +55,7 @@
- Update python:3.12-slim Docker digest to 2a6386a ([#3284](https://github.com/nf-core/tools/pull/3284))
- Update pre-commit hook astral-sh/ruff-pre-commit to v0.8.0 ([#3299](https://github.com/nf-core/tools/pull/3299))
- Update gitpod/workspace-base Docker digest to 12853f7 ([#3309](https://github.com/nf-core/tools/pull/3309))
- Run pre-commit when testing linting the template pipeline ([#3280](https://github.com/nf-core/tools/pull/3280))

## [v3.0.2 - Titanium Tapir Patch](https://github.com/nf-core/tools/releases/tag/3.0.2) - [2024-10-11]

Expand Down
4 changes: 2 additions & 2 deletions nf_core/components/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from nf_core.components.nfcore_component import NFCoreComponent
from nf_core.modules.modules_json import ModulesJson
from nf_core.pipelines.lint_utils import console
from nf_core.utils import LintConfigType
from nf_core.utils import NFCoreYamlLintConfig
from nf_core.utils import plural_s as _s

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -80,7 +80,7 @@ def __init__(
self.failed: List[LintResult] = []
self.all_local_components: List[NFCoreComponent] = []

self.lint_config: Optional[LintConfigType] = None
self.lint_config: Optional[NFCoreYamlLintConfig] = None
self.modules_json: Optional[ModulesJson] = None

if self.component_type == "modules":
Expand Down
16 changes: 8 additions & 8 deletions nf_core/pipelines/create/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import re
import shutil
from pathlib import Path
from typing import Dict, List, Optional, Tuple, Union, cast
from typing import Dict, List, Optional, Tuple, Union

import git
import git.config
Expand All @@ -22,7 +22,7 @@
from nf_core.pipelines.create_logo import create_logo
from nf_core.pipelines.lint_utils import run_prettier_on_file
from nf_core.pipelines.rocrate import ROCrate
from nf_core.utils import LintConfigType, NFCoreTemplateConfig
from nf_core.utils import NFCoreTemplateConfig, NFCoreYamlLintConfig

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -68,7 +68,7 @@ def __init__(
_, config_yml = nf_core.utils.load_tools_config(outdir if outdir else Path().cwd())
# Obtain a CreateConfig object from `.nf-core.yml` config file
if config_yml is not None and getattr(config_yml, "template", None) is not None:
self.config = CreateConfig(**config_yml["template"].model_dump())
self.config = CreateConfig(**config_yml["template"].model_dump(exclude_none=True))
else:
raise UserWarning("The template configuration was not provided in '.nf-core.yml'.")
# Update the output directory
Expand Down Expand Up @@ -206,7 +206,7 @@ def obtain_jinja_params_dict(
config_yml = None

# Set the parameters for the jinja template
jinja_params = self.config.model_dump()
jinja_params = self.config.model_dump(exclude_none=True)

# Add template areas to jinja params and create list of areas with paths to skip
skip_areas = []
Expand Down Expand Up @@ -369,8 +369,8 @@ def render_template(self) -> None:
config_fn, config_yml = nf_core.utils.load_tools_config(self.outdir)
if config_fn is not None and config_yml is not None:
with open(str(config_fn), "w") as fh:
config_yml.template = NFCoreTemplateConfig(**self.config.model_dump())
yaml.safe_dump(config_yml.model_dump(), fh)
config_yml.template = NFCoreTemplateConfig(**self.config.model_dump(exclude_none=True))
yaml.safe_dump(config_yml.model_dump(exclude_none=True), fh)
log.debug(f"Dumping pipeline template yml to pipeline config file '{config_fn.name}'")

# Run prettier on files
Expand Down Expand Up @@ -401,9 +401,9 @@ def fix_linting(self):
# Add the lint content to the preexisting nf-core config
config_fn, nf_core_yml = nf_core.utils.load_tools_config(self.outdir)
if config_fn is not None and nf_core_yml is not None:
nf_core_yml.lint = cast(LintConfigType, lint_config)
nf_core_yml.lint = NFCoreYamlLintConfig(**lint_config)
with open(self.outdir / config_fn, "w") as fh:
yaml.dump(nf_core_yml.model_dump(), fh, default_flow_style=False, sort_keys=False)
yaml.dump(nf_core_yml.model_dump(exclude_none=True), fh, default_flow_style=False, sort_keys=False)

def make_pipeline_logo(self):
"""Fetch a logo for the new pipeline from the nf-core website"""
Expand Down
14 changes: 6 additions & 8 deletions nf_core/pipelines/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
from nf_core import __version__
from nf_core.components.lint import ComponentLint
from nf_core.pipelines.lint_utils import console
from nf_core.utils import NFCoreYamlConfig, NFCoreYamlLintConfig, strip_ansi_codes
from nf_core.utils import plural_s as _s
from nf_core.utils import strip_ansi_codes

from .actions_awsfulltest import actions_awsfulltest
from .actions_awstest import actions_awstest
Expand Down Expand Up @@ -112,7 +112,7 @@ def __init__(
# Initialise the parent object
super().__init__(wf_path)

self.lint_config = {}
self.lint_config: Optional[NFCoreYamlLintConfig] = None
self.release_mode = release_mode
self.fail_ignored = fail_ignored
self.fail_warned = fail_warned
Expand Down Expand Up @@ -173,13 +173,12 @@ def _load_lint_config(self) -> bool:
Add parsed config to the `self.lint_config` class attribute.
"""
_, tools_config = nf_core.utils.load_tools_config(self.wf_path)
self.lint_config = getattr(tools_config, "lint", {}) or {}
self.lint_config = getattr(tools_config, "lint", None) or None
is_correct = True

# Check if we have any keys that don't match lint test names
if self.lint_config is not None:
for k in self.lint_config:
if k != "nfcore_components" and k not in self.lint_tests:
for k, v in self.lint_config:
if v is not None and k != "nfcore_components" and k not in self.lint_tests:
# nfcore_components is an exception to allow custom pipelines without nf-core components
log.warning(f"Found unrecognised test name '{k}' in pipeline lint config")
is_correct = False
Expand Down Expand Up @@ -594,7 +593,7 @@ def run_linting(
lint_obj._load_lint_config()
lint_obj.load_pipeline_config()

if "nfcore_components" in lint_obj.lint_config and not lint_obj.lint_config["nfcore_components"]:
if lint_obj.lint_config and not lint_obj.lint_config["nfcore_components"]:
module_lint_obj = None
subworkflow_lint_obj = None
else:
Expand Down Expand Up @@ -679,5 +678,4 @@ def run_linting(
if len(lint_obj.failed) > 0:
if release_mode:
log.info("Reminder: Lint tests were run in --release mode.")

return lint_obj, module_lint_obj, subworkflow_lint_obj
9 changes: 9 additions & 0 deletions nf_core/pipelines/lint/multiqc_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ def multiqc_config(self) -> Dict[str, List[str]]:
lint:
multiqc_config: False
To disable this test only for specific sections, you can specify a list of section names.
For example:
.. code-block:: yaml
lint:
multiqc_config:
- report_section_order
- report_comment
"""

passed: List[str] = []
Expand Down
31 changes: 16 additions & 15 deletions nf_core/pipelines/lint/nfcore_yml.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import re
from pathlib import Path
from typing import Dict, List

from ruamel.yaml import YAML

from nf_core import __version__

REPOSITORY_TYPES = ["pipeline", "modules"]
Expand All @@ -26,21 +27,23 @@ def nfcore_yml(self) -> Dict[str, List[str]]:
failed: List[str] = []
ignored: List[str] = []

yaml = YAML()

# Remove field that should be ignored according to the linting config
ignore_configs = self.lint_config.get(".nf-core", []) if self.lint_config is not None else []
try:
with open(Path(self.wf_path, ".nf-core.yml")) as fh:
content = fh.read()
except FileNotFoundError:
with open(Path(self.wf_path, ".nf-core.yaml")) as fh:
content = fh.read()
for ext in (".yml", ".yaml"):
try:
nf_core_yml = yaml.load(Path(self.wf_path) / f".nf-core{ext}")
break
except FileNotFoundError:
continue
else:
raise FileNotFoundError("No `.nf-core.yml` file found.")

if "repository_type" not in ignore_configs:
# Check that the repository type is set in the .nf-core.yml
repo_type_re = r"repository_type: (.+)"
match = re.search(repo_type_re, content)
if match:
repo_type = match.group(1)
if "repository_type" in nf_core_yml:
repo_type = nf_core_yml["repository_type"]
if repo_type not in REPOSITORY_TYPES:
failed.append(
f"Repository type in `.nf-core.yml` is not valid. "
Expand All @@ -55,10 +58,8 @@ def nfcore_yml(self) -> Dict[str, List[str]]:

if "nf_core_version" not in ignore_configs:
# Check that the nf-core version is set in the .nf-core.yml
nf_core_version_re = r"nf_core_version: (.+)"
match = re.search(nf_core_version_re, content)
if match:
nf_core_version = match.group(1).strip('"')
if "nf_core_version" in nf_core_yml:
nf_core_version = nf_core_yml["nf_core_version"]
if nf_core_version != __version__ and "dev" not in nf_core_version:
warned.append(
f"nf-core version in `.nf-core.yml` is not set to the latest version. "
Expand Down
15 changes: 15 additions & 0 deletions nf_core/pipelines/lint/readme.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ def readme(self):
* If pipeline is released but still contains a 'zenodo.XXXXXXX' tag, the test fails
To disable this test, add the following to the pipeline's ``.nf-core.yml`` file:
.. code-block:: yaml
lint:
readme: False
To disable subsets of these tests, add the following to the pipeline's ``.nf-core.yml`` file:
.. code-block:: yaml
lint:
readme:
- nextflow_badge
- zenodo_release
"""
passed = []
warned = []
Expand Down
2 changes: 1 addition & 1 deletion nf_core/pipelines/lint/template_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ def template_strings(self):
ignored = []
# Files that should be ignored according to the linting config
ignore_files = self.lint_config.get("template_strings", []) if self.lint_config is not None else []
files = self.list_files()

files = self.list_files()
# Loop through files, searching for string
num_matches = 0
for fn in files:
Expand Down
42 changes: 25 additions & 17 deletions nf_core/pipelines/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import re
import shutil
from pathlib import Path
from typing import Dict, Optional, Union
from typing import Any, Dict, Optional, Tuple, Union

import git
import questionary
Expand Down Expand Up @@ -105,7 +105,7 @@ def __init__(
with open(template_yaml_path) as f:
self.config_yml.template = yaml.safe_load(f)
with open(self.config_yml_path, "w") as fh:
yaml.safe_dump(self.config_yml.model_dump(), fh)
yaml.safe_dump(self.config_yml.model_dump(exclude_none=True), fh)
log.info(f"Saved pipeline creation settings to '{self.config_yml_path}'")
raise SystemExit(
f"Please commit your changes and delete the {template_yaml_path} file. Then run the sync command again."
Expand All @@ -120,7 +120,7 @@ def __init__(
requests.auth.HTTPBasicAuth(self.gh_username, os.environ["GITHUB_AUTH_TOKEN"])
)

def sync(self):
def sync(self) -> None:
"""Find workflow attributes, create a new template pipeline on TEMPLATE"""

# Clear requests_cache so that we don't get stale API responses
Expand Down Expand Up @@ -271,7 +271,7 @@ def make_template_pipeline(self):

self.config_yml.template.force = True
with open(self.config_yml_path, "w") as config_path:
yaml.safe_dump(self.config_yml.model_dump(), config_path)
yaml.safe_dump(self.config_yml.model_dump(exclude_none=True), config_path)

try:
pipeline_create_obj = nf_core.pipelines.create.create.PipelineCreate(
Expand All @@ -291,7 +291,7 @@ def make_template_pipeline(self):
self.config_yml.template.outdir = "."
# Update nf-core version
self.config_yml.nf_core_version = nf_core.__version__
dump_yaml_with_prettier(self.config_yml_path, self.config_yml.model_dump())
dump_yaml_with_prettier(self.config_yml_path, self.config_yml.model_dump(exclude_none=True))

except Exception as err:
# Reset to where you were to prevent git getting messed up.
Expand Down Expand Up @@ -416,12 +416,8 @@ def close_open_template_merge_prs(self):
list_prs_url = f"https://api.github.com/repos/{self.gh_repo}/pulls"
with self.gh_api.cache_disabled():
list_prs_request = self.gh_api.get(list_prs_url)
try:
list_prs_json = json.loads(list_prs_request.content)
list_prs_pp = json.dumps(list_prs_json, indent=4)
except Exception:
list_prs_json = list_prs_request.content
list_prs_pp = list_prs_request.content

list_prs_json, list_prs_pp = self._parse_json_response(list_prs_request)

log.debug(f"GitHub API listing existing PRs:\n{list_prs_url}\n{list_prs_pp}")
if list_prs_request.status_code != 200:
Expand Down Expand Up @@ -462,12 +458,8 @@ def close_open_pr(self, pr) -> bool:
# Update the PR status to be closed
with self.gh_api.cache_disabled():
pr_request = self.gh_api.patch(url=pr["url"], data=json.dumps({"state": "closed"}))
try:
pr_request_json = json.loads(pr_request.content)
pr_request_pp = json.dumps(pr_request_json, indent=4)
except Exception:
pr_request_json = pr_request.content
pr_request_pp = pr_request.content

pr_request_json, pr_request_pp = self._parse_json_response(pr_request)

# PR update worked
if pr_request.status_code == 200:
Expand All @@ -481,6 +473,22 @@ def close_open_pr(self, pr) -> bool:
log.warning(f"Could not close PR ('{pr_request.status_code}'):\n{pr['url']}\n{pr_request_pp}")
return False

@staticmethod
def _parse_json_response(response) -> Tuple[Any, str]:
"""Helper method to parse JSON response and create pretty-printed string.
Args:
response: requests.Response object
Returns:
Tuple of (parsed_json, pretty_printed_str)
"""
try:
json_data = json.loads(response.content)
return json_data, json.dumps(json_data, indent=4)
except Exception:
return response.content, str(response.content)

def reset_target_dir(self):
"""
Reset the target pipeline directory. Check out the original branch.
Expand Down
Loading

0 comments on commit a7351a2

Please sign in to comment.