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

allow mixed str and dict in lint config #3228

Merged
merged 25 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5bc36e4
allow mixed list and dict in lint config
mashehu Oct 17, 2024
81bdb3b
nested too deeply
mashehu Oct 17, 2024
3d8d4b7
[automated] Update CHANGELOG.md
nf-core-bot Oct 17, 2024
2b4029b
handle new nf-core.yml structure
mashehu Oct 21, 2024
3a55f36
update documentation for `multiqc_config` linting
mashehu Oct 21, 2024
fcc442a
parse yaml correctly
mashehu Oct 21, 2024
482f9f9
found a better way to handle the ignore file being None
mashehu Oct 21, 2024
663a932
handle new lint config structure
mashehu Oct 21, 2024
53ae873
add tests with different valid yaml structures
mashehu Oct 21, 2024
b5a546c
Merge branch 'fix-yml-config-type' of github.com:mashehu/tools into f…
mashehu Oct 21, 2024
57f7ca8
remove last traces of LintConfigType
mashehu Oct 21, 2024
e743185
fix incorrect type
mashehu Oct 21, 2024
58869a1
more type fixes
mashehu Oct 21, 2024
afbd51b
add all lint tests to config
mashehu Oct 22, 2024
9bf91f5
switch all defaults to None and drop them on dump
mashehu Oct 22, 2024
61bb733
drop None values when checking for test names
mashehu Oct 22, 2024
e2ac2b5
fix test_lint tests
mashehu Oct 22, 2024
befd38e
Merge branch 'dev' of github.com:nf-core/tools into fix-yml-config-type
mashehu Oct 23, 2024
b7d52a9
Merge branch 'fix-yml-config-type' of github.com:mashehu/tools into f…
mashehu Oct 23, 2024
c63f3be
Merge branch 'dev' of github.com:nf-core/tools into fix-yml-config-type
mashehu Dec 2, 2024
86b926b
test also the main sync function itsel
mashehu Dec 2, 2024
ee86c15
combine json parsing code
mashehu Dec 2, 2024
10e691b
remove broken test
mashehu Dec 2, 2024
11f7f42
fix type error
mashehu Dec 2, 2024
eed0598
Apply suggestions from code review
mashehu Dec 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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 @@ -52,6 +52,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 lint_obj.lint_config["nfcore_components"] is False:
mashehu marked this conversation as resolved.
Show resolved Hide resolved
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
mashehu marked this conversation as resolved.
Show resolved Hide resolved

"""
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
Loading