From 64c684ca4d91edc34448d858a448bc2dde8861d3 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 12:29:00 +0200 Subject: [PATCH 01/28] increase worflow linting --- planemo/commands/cmd_workflow_lint.py | 6 ++ planemo/lint.py | 2 + planemo/workflow_lint.py | 92 +++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 80085c46d..64f9c1cc6 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -14,6 +14,12 @@ @options.report_xunit() @options.fail_level_option() @options.skip_option() +@click.option( + "--iwc", + is_flag=True, + default=False, + help="Check workflows directory with the standards of iwc", +) @command_function def cli(ctx, paths, **kwds): """Check workflows for syntax errors and best practices.""" diff --git a/planemo/lint.py b/planemo/lint.py index 5b85022de..d0c302f50 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -18,6 +18,7 @@ def build_lint_args(ctx, **kwds): """Handle common report, error, and skip linting arguments.""" report_level = kwds.get("report_level", "all") fail_level = kwds.get("fail_level", "warn") + iwc_grade = kwds.get("iwc", False) skip = kwds.get("skip", None) if skip is None: skip = ctx.global_config.get("lint_skip", "") @@ -42,6 +43,7 @@ def build_lint_args(ctx, **kwds): level=report_level, fail_level=fail_level, skip_types=skip_types, + iwc_grade=iwc_grade, ) return lint_args diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index fdc2bbc80..f34f0195e 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -57,6 +57,8 @@ class WorkflowLintContext(LintContext): # Setup training topic for linting - probably should pass this through # from click arguments. training_topic = None + release = None + iwc_grade = False def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: @@ -138,6 +140,13 @@ def lint_workflow_artifacts_on_paths( def _lint_workflow_artifacts_on_path( lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Union[str, List[str]]] ) -> None: + if lint_args['iwc_grade']: + if not os.path.isdir(path): + raise ValueError("iwc standards can only be checked on directories.") + lint_context.lint("lint_required_files", _lint_required_files_workflow_dir, path) + lint_context.lint("lint_changelog", _lint_changelog_version, path) + lint_context.iwc_grade = True + for potential_workflow_artifact_path in find_potential_workflow_files(path): if os.path.basename(potential_workflow_artifact_path) == DOCKSTORE_REGISTRY_CONF: lint_context.lint("lint_dockstore", _lint_dockstore_config, potential_workflow_artifact_path) @@ -150,8 +159,10 @@ def structure(path, lint_context): workflow_class = workflow_dict.get("class") lint_func = lint_format2 if workflow_class == "GalaxyWorkflow" else lint_ga lint_func(lint_context, workflow_dict, path=path) - + lint_context.lint("lint_structure", structure, potential_workflow_artifact_path) + if lint_context.iwc_grade and lint_context.version is not None: + lint_context.lint("lint_version", check_version, potential_workflow_artifact_path) lint_context.lint("lint_best_practices", _lint_best_practices, potential_workflow_artifact_path) lint_context.lint("lint_tests", _lint_tsts, potential_workflow_artifact_path) lint_context.lint("lint_tool_ids", _lint_tool_ids, potential_workflow_artifact_path) @@ -394,6 +405,11 @@ def _lint_dockstore_config(path: str, lint_context: WorkflowLintContext) -> None lint_context.error("Invalid YAML contents found in %s" % DOCKSTORE_REGISTRY_CONF) return + if "version" not in dockstore_yaml: + lint_context.error("Invalid YAML contents found in %s, no version defined" % DOCKSTORE_REGISTRY_CONF) + if dockstore_yaml.get("version") != DOCKSTORE_REGISTRY_CONF_VERSION: + lint_context.error("Invalid YAML version found in %s." % DOCKSTORE_REGISTRY_CONF) + if "workflows" not in dockstore_yaml: lint_context.error("Invalid YAML contents found in %s, no workflows defined" % DOCKSTORE_REGISTRY_CONF) return @@ -403,8 +419,16 @@ def _lint_dockstore_config(path: str, lint_context: WorkflowLintContext) -> None lint_context.error("Invalid YAML contents found in %s, workflows not a list" % DOCKSTORE_REGISTRY_CONF) return + if len(workflow_entries) == 0: + lint_context.error("No workflow specified in the .dockstore.yml.") + + workflow_names_in_dockstore = [] for workflow_entry in workflow_entries: _lint_dockstore_workflow_entry(lint_context, os.path.dirname(path), workflow_entry) + workflow_name = workflow_entry.get("name") + if workflow_name in workflow_names_in_dockstore: + lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} has multiple workflow entries with the same name") + workflow_names_in_dockstore.append(workflow_name) def _lint_dockstore_workflow_entry( @@ -415,20 +439,26 @@ def _lint_dockstore_workflow_entry( return found_errors = False - for required_key in ["primaryDescriptorPath", "subclass"]: + for required_key in ["primaryDescriptorPath", "subclass", "name"]: if required_key not in workflow_entry: lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing required key {required_key}") found_errors = True + if lint_context.iwc_grade: + lint_fun = lint_context.error + else: + lint_fun = lint_context.warn for recommended_key in ["testParameterFiles"]: if recommended_key not in workflow_entry: - lint_context.warn(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}") + lint_fun(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}") if found_errors: # Don't do the rest of the validation for a broken file. return - # TODO: validate subclass + if workflow_entry.get("subclass") != "Galaxy": + lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry subclass must be 'Galaxy'.") + descriptor_path = workflow_entry["primaryDescriptorPath"] test_files = workflow_entry.get("testParameterFiles", []) @@ -437,6 +467,24 @@ def _lint_dockstore_workflow_entry( if not os.path.exists(referenced_path): lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry references absent file {referenced_file}") + # Check there is no space in name: + workflow_name = workflow_entry.get("name") + # Check the name has no space + if ' ' in workflow_name: + lint_context.error("Dockstore does not accept workflow names with space.", + f"Change '{workflow_name}' in {DOCKSTORE_REGISTRY_CONF}.") + + # Check there is at least one author + if len(workflow_entry.get('authors')) == 0: + lint_fun(f"Workflow {workflow_name} should have no " \ + "'authors' in the .dockstore.yml.") + # Check there is not mailto + for author in workflow_entry.get('authors'): + if 'email' in author: + if author['email'].startswith('mailto:'): + lint_context.error("email field of the .dockstore.yml must not " + "contain 'mailto:'") + def looks_like_a_workflow(path: str) -> bool: """Return boolean indicating if this path looks like a workflow.""" @@ -537,3 +585,39 @@ def _lint_tool_ids_steps(lint_context: WorkflowLintContext, wf_dict: Dict, ts: T if not failed: lint_context.valid("All tool ids appear to be valid.") return None + +def _lint_required_files_workflow_dir(path: str, lint_context: WorkflowLintContext) -> None: + # Check all required files are present + required_files = \ + ['README.md', 'CHANGELOG.md', '.dockstore.yml'] + for required_file in required_files: + if not os.path.exists(os.path.join(path, required_file)): + lint_context.error(f"The file {required_file} is" \ + " missing but required.") + +def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> None: + lint_context.version = None + # Check the version can be get from the CHANGELOG.md + if not os.path.exists(os.path.join(path, 'CHANGELOG.md')): + return + with open(os.path.join(path, 'CHANGELOG.md'), 'r') as f: + for line in f: + if line.startswith('## ['): + lint_context.version = line.split(']')[0].replace('## [', '') + break + + if lint_context.version is None: + lint_context.error("No version found in CHANGELOG. " \ + "The version should be in a line that starts like " \ + "'## [version number]'") + + +def check_version(path, lint_context): + with open(path) as f: + workflow_dict = ordered_load(f) + if 'release' not in workflow_dict: + lint_context.error(f"The workflow {path} has no release") + else: + if lint_context.version is not None and workflow_dict.get("release") != lint_context.version: + lint_context.error(f"The release of workflow {file} does not match " \ + "the version in the CHANGELOG.") From af6758e5e81e60ed02edbbbeef868afd76dc0842 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 12:39:22 +0200 Subject: [PATCH 02/28] linting --- planemo/workflow_lint.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index f34f0195e..487311d49 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -159,7 +159,7 @@ def structure(path, lint_context): workflow_class = workflow_dict.get("class") lint_func = lint_format2 if workflow_class == "GalaxyWorkflow" else lint_ga lint_func(lint_context, workflow_dict, path=path) - + lint_context.lint("lint_structure", structure, potential_workflow_artifact_path) if lint_context.iwc_grade and lint_context.version is not None: lint_context.lint("lint_version", check_version, potential_workflow_artifact_path) @@ -476,14 +476,13 @@ def _lint_dockstore_workflow_entry( # Check there is at least one author if len(workflow_entry.get('authors')) == 0: - lint_fun(f"Workflow {workflow_name} should have no " \ - "'authors' in the .dockstore.yml.") + lint_fun(f"Workflow {workflow_name} should have no " + "'authors' in the .dockstore.yml.") # Check there is not mailto for author in workflow_entry.get('authors'): - if 'email' in author: - if author['email'].startswith('mailto:'): - lint_context.error("email field of the .dockstore.yml must not " - "contain 'mailto:'") + if author.get('email', '').startswith('mailto:'): + lint_context.error("email field of the .dockstore.yml must not " + "contain 'mailto:'") def looks_like_a_workflow(path: str) -> bool: @@ -586,14 +585,16 @@ def _lint_tool_ids_steps(lint_context: WorkflowLintContext, wf_dict: Dict, ts: T lint_context.valid("All tool ids appear to be valid.") return None + def _lint_required_files_workflow_dir(path: str, lint_context: WorkflowLintContext) -> None: # Check all required files are present required_files = \ ['README.md', 'CHANGELOG.md', '.dockstore.yml'] for required_file in required_files: if not os.path.exists(os.path.join(path, required_file)): - lint_context.error(f"The file {required_file} is" \ - " missing but required.") + lint_context.error(f"The file {required_file} is" + " missing but required.") + def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> None: lint_context.version = None @@ -607,8 +608,8 @@ def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> Non break if lint_context.version is None: - lint_context.error("No version found in CHANGELOG. " \ - "The version should be in a line that starts like " \ + lint_context.error("No version found in CHANGELOG. " + "The version should be in a line that starts like " "'## [version number]'") @@ -619,5 +620,5 @@ def check_version(path, lint_context): lint_context.error(f"The workflow {path} has no release") else: if lint_context.version is not None and workflow_dict.get("release") != lint_context.version: - lint_context.error(f"The release of workflow {file} does not match " \ - "the version in the CHANGELOG.") + lint_context.error(f"The release of workflow {path} does not match " + "the version in the CHANGELOG.") From 1d61ca1b8c0c05909a49fcf72daa2cc771f07fc8 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 14:38:40 +0200 Subject: [PATCH 03/28] restore build_lint_args --- planemo/commands/cmd_workflow_lint.py | 1 + planemo/lint.py | 2 -- planemo/workflow_lint.py | 6 +++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 64f9c1cc6..d226f40f7 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -29,5 +29,6 @@ def cli(ctx, paths, **kwds): ctx, paths, lint_args, + iwc_grade = kwds.get("iwc", False) ) ctx.exit(exit_code) diff --git a/planemo/lint.py b/planemo/lint.py index d0c302f50..5b85022de 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -18,7 +18,6 @@ def build_lint_args(ctx, **kwds): """Handle common report, error, and skip linting arguments.""" report_level = kwds.get("report_level", "all") fail_level = kwds.get("fail_level", "warn") - iwc_grade = kwds.get("iwc", False) skip = kwds.get("skip", None) if skip is None: skip = ctx.global_config.get("lint_skip", "") @@ -43,7 +42,6 @@ def build_lint_args(ctx, **kwds): level=report_level, fail_level=fail_level, skip_types=skip_types, - iwc_grade=iwc_grade, ) return lint_args diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 487311d49..12ad9f690 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -124,10 +124,11 @@ def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: def lint_workflow_artifacts_on_paths( - ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]] + ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]], iwc_grade: bool ) -> int: report_level = lint_args["level"] lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"]) + lint_context.iwc_grade = iwc_grade for path in paths: _lint_workflow_artifacts_on_path(lint_context, path, lint_args) @@ -140,12 +141,11 @@ def lint_workflow_artifacts_on_paths( def _lint_workflow_artifacts_on_path( lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Union[str, List[str]]] ) -> None: - if lint_args['iwc_grade']: + if lint_context.iwc_grade: if not os.path.isdir(path): raise ValueError("iwc standards can only be checked on directories.") lint_context.lint("lint_required_files", _lint_required_files_workflow_dir, path) lint_context.lint("lint_changelog", _lint_changelog_version, path) - lint_context.iwc_grade = True for potential_workflow_artifact_path in find_potential_workflow_files(path): if os.path.basename(potential_workflow_artifact_path) == DOCKSTORE_REGISTRY_CONF: From dd9b1bb227dfac02e30571307c4735b6cb784d2d Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 14:57:21 +0200 Subject: [PATCH 04/28] fix cases with no authors in dockstore.yml --- planemo/workflow_lint.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 12ad9f690..dbcc94b74 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -475,11 +475,11 @@ def _lint_dockstore_workflow_entry( f"Change '{workflow_name}' in {DOCKSTORE_REGISTRY_CONF}.") # Check there is at least one author - if len(workflow_entry.get('authors')) == 0: - lint_fun(f"Workflow {workflow_name} should have no " + if len(workflow_entry.get('authors', [])) == 0: + lint_fun(f"Workflow {workflow_name} have no " "'authors' in the .dockstore.yml.") # Check there is not mailto - for author in workflow_entry.get('authors'): + for author in workflow_entry.get('authors', []): if author.get('email', '').startswith('mailto:'): lint_context.error("email field of the .dockstore.yml must not " "contain 'mailto:'") From f7401d8fe7b2df86e731c90755f8e880d43f25e1 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 14:57:45 +0200 Subject: [PATCH 05/28] fix DOCKSTORE_REGISTRY_CONF_VERSION --- planemo/workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index dbcc94b74..6c1c2fea5 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -407,7 +407,7 @@ def _lint_dockstore_config(path: str, lint_context: WorkflowLintContext) -> None if "version" not in dockstore_yaml: lint_context.error("Invalid YAML contents found in %s, no version defined" % DOCKSTORE_REGISTRY_CONF) - if dockstore_yaml.get("version") != DOCKSTORE_REGISTRY_CONF_VERSION: + if str(dockstore_yaml.get("version")) != DOCKSTORE_REGISTRY_CONF_VERSION: lint_context.error("Invalid YAML version found in %s." % DOCKSTORE_REGISTRY_CONF) if "workflows" not in dockstore_yaml: From d4ae50fc7155cd9319bd2a0a869fd8eb944141f7 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 14:58:07 +0200 Subject: [PATCH 06/28] change name from required to recommanded --- planemo/workflow_lint.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 6c1c2fea5..116ff8872 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -425,7 +425,7 @@ def _lint_dockstore_config(path: str, lint_context: WorkflowLintContext) -> None workflow_names_in_dockstore = [] for workflow_entry in workflow_entries: _lint_dockstore_workflow_entry(lint_context, os.path.dirname(path), workflow_entry) - workflow_name = workflow_entry.get("name") + workflow_name = workflow_entry.get("name", "") if workflow_name in workflow_names_in_dockstore: lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} has multiple workflow entries with the same name") workflow_names_in_dockstore.append(workflow_name) @@ -439,7 +439,7 @@ def _lint_dockstore_workflow_entry( return found_errors = False - for required_key in ["primaryDescriptorPath", "subclass", "name"]: + for required_key in ["primaryDescriptorPath", "subclass"]: if required_key not in workflow_entry: lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing required key {required_key}") found_errors = True @@ -448,7 +448,7 @@ def _lint_dockstore_workflow_entry( lint_fun = lint_context.error else: lint_fun = lint_context.warn - for recommended_key in ["testParameterFiles"]: + for recommended_key in ["testParameterFiles", "name"]: if recommended_key not in workflow_entry: lint_fun(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}") From 619cae9ce20f7881a89ddf73e80069293a8d3140 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 15:01:58 +0200 Subject: [PATCH 07/28] linting --- planemo/commands/cmd_workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index d226f40f7..79ac6d305 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -29,6 +29,6 @@ def cli(ctx, paths, **kwds): ctx, paths, lint_args, - iwc_grade = kwds.get("iwc", False) + iwc_grade=kwds.get("iwc", False) ) ctx.exit(exit_code) From eb86e3b8bed425802990da90bb827e6d474777c7 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Sat, 29 Jun 2024 15:12:20 +0200 Subject: [PATCH 08/28] fix forgotten consequence of name from required to recommanded --- planemo/workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 116ff8872..91ed64f29 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -468,7 +468,7 @@ def _lint_dockstore_workflow_entry( lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry references absent file {referenced_file}") # Check there is no space in name: - workflow_name = workflow_entry.get("name") + workflow_name = workflow_entry.get("name", "") # Check the name has no space if ' ' in workflow_name: lint_context.error("Dockstore does not accept workflow names with space.", From 5516443ef37d30ef6e94fca8055db0f44733ecda Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 10:26:33 +0200 Subject: [PATCH 09/28] put iwc_grade in lint_args as suggested by @bernt-matthias --- planemo/commands/cmd_workflow_lint.py | 4 ++-- planemo/workflow_lint.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 79ac6d305..132e84d99 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -25,10 +25,10 @@ def cli(ctx, paths, **kwds): """Check workflows for syntax errors and best practices.""" # Unlike tools, lets just make this recursive by default. lint_args = build_lint_args(ctx, **kwds) + lint_args["iwc_grade"] = kwds.get("iwc", False) exit_code = lint_workflow_artifacts_on_paths( ctx, paths, - lint_args, - iwc_grade=kwds.get("iwc", False) + lint_args ) ctx.exit(exit_code) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 91ed64f29..91346e4d2 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -124,11 +124,11 @@ def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: def lint_workflow_artifacts_on_paths( - ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]], iwc_grade: bool + ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]] ) -> int: report_level = lint_args["level"] lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"]) - lint_context.iwc_grade = iwc_grade + lint_context.iwc_grade = lint_args["iwc_grade"] for path in paths: _lint_workflow_artifacts_on_path(lint_context, path, lint_args) From 8639d1dc9adb5de58fa344ec866e64e2c6277221 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 11:45:23 +0200 Subject: [PATCH 10/28] remove release/version from WorkflowLintContext as suggested by @matthias-bernt --- planemo/workflow_lint.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 91346e4d2..c618dfe58 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -57,7 +57,6 @@ class WorkflowLintContext(LintContext): # Setup training topic for linting - probably should pass this through # from click arguments. training_topic = None - release = None iwc_grade = False @@ -161,8 +160,8 @@ def structure(path, lint_context): lint_func(lint_context, workflow_dict, path=path) lint_context.lint("lint_structure", structure, potential_workflow_artifact_path) - if lint_context.iwc_grade and lint_context.version is not None: - lint_context.lint("lint_version", check_version, potential_workflow_artifact_path) + if lint_context.iwc_grade: + lint_context.lint("lint_release", _lint_release, potential_workflow_artifact_path) lint_context.lint("lint_best_practices", _lint_best_practices, potential_workflow_artifact_path) lint_context.lint("lint_tests", _lint_tsts, potential_workflow_artifact_path) lint_context.lint("lint_tool_ids", _lint_tool_ids, potential_workflow_artifact_path) @@ -596,29 +595,37 @@ def _lint_required_files_workflow_dir(path: str, lint_context: WorkflowLintConte " missing but required.") -def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> None: - lint_context.version = None - # Check the version can be get from the CHANGELOG.md +def _get_changelog_version(path: str) -> str: + # Get the version from the CHANGELOG.md + version = "" if not os.path.exists(os.path.join(path, 'CHANGELOG.md')): - return + return version with open(os.path.join(path, 'CHANGELOG.md'), 'r') as f: for line in f: if line.startswith('## ['): - lint_context.version = line.split(']')[0].replace('## [', '') + version = line.split(']')[0].replace('## [', '') break + return version + - if lint_context.version is None: +def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> None: + # Check the version can be get from the CHANGELOG.md + if not os.path.exists(os.path.join(path, 'CHANGELOG.md')): + return + if _get_changelog_version(path) == "": lint_context.error("No version found in CHANGELOG. " "The version should be in a line that starts like " "'## [version number]'") -def check_version(path, lint_context): +def _lint_release(path, lint_context): with open(path) as f: workflow_dict = ordered_load(f) if 'release' not in workflow_dict: lint_context.error(f"The workflow {path} has no release") else: - if lint_context.version is not None and workflow_dict.get("release") != lint_context.version: + # Try to get the version from the CHANGELOG: + version = _get_changelog_version(os.path.join(os.path.dirname(path), "CHANGELOG.md")) + if version != "" and workflow_dict.get("release") != version: lint_context.error(f"The release of workflow {path} does not match " "the version in the CHANGELOG.") From 5c19e56d2dc64a1d71c2ce1abb44bc33415197b7 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 12:00:06 +0200 Subject: [PATCH 11/28] fix type of lint_args --- planemo/commands/cmd_workflow_lint.py | 2 +- planemo/workflow_lint.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 132e84d99..5cdf0c4e1 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -25,7 +25,7 @@ def cli(ctx, paths, **kwds): """Check workflows for syntax errors and best practices.""" # Unlike tools, lets just make this recursive by default. lint_args = build_lint_args(ctx, **kwds) - lint_args["iwc_grade"] = kwds.get("iwc", False) + lint_args["iwc_grade"] = str(kwds.get("iwc", False)) exit_code = lint_workflow_artifacts_on_paths( ctx, paths, diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index c618dfe58..81369c2cf 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -127,7 +127,7 @@ def lint_workflow_artifacts_on_paths( ) -> int: report_level = lint_args["level"] lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"]) - lint_context.iwc_grade = lint_args["iwc_grade"] + lint_context.iwc_grade = lint_args["iwc_grade"] == "True" for path in paths: _lint_workflow_artifacts_on_path(lint_context, path, lint_args) From 302c504722c9d3edfc9e0b8a22b5c58e8ff9e3e6 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 13:01:59 +0200 Subject: [PATCH 12/28] remove iwc_grade from WorkflowLintContext and add lint_dockstore_best_practices --- planemo/workflow_lint.py | 54 ++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 81369c2cf..389457199 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -57,7 +57,6 @@ class WorkflowLintContext(LintContext): # Setup training topic for linting - probably should pass this through # from click arguments. training_topic = None - iwc_grade = False def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: @@ -127,7 +126,6 @@ def lint_workflow_artifacts_on_paths( ) -> int: report_level = lint_args["level"] lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"]) - lint_context.iwc_grade = lint_args["iwc_grade"] == "True" for path in paths: _lint_workflow_artifacts_on_path(lint_context, path, lint_args) @@ -140,7 +138,8 @@ def lint_workflow_artifacts_on_paths( def _lint_workflow_artifacts_on_path( lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Union[str, List[str]]] ) -> None: - if lint_context.iwc_grade: + iwc_grade = lint_args["iwc_grade"] == "True" + if iwc_grade: if not os.path.isdir(path): raise ValueError("iwc standards can only be checked on directories.") lint_context.lint("lint_required_files", _lint_required_files_workflow_dir, path) @@ -149,6 +148,8 @@ def _lint_workflow_artifacts_on_path( for potential_workflow_artifact_path in find_potential_workflow_files(path): if os.path.basename(potential_workflow_artifact_path) == DOCKSTORE_REGISTRY_CONF: lint_context.lint("lint_dockstore", _lint_dockstore_config, potential_workflow_artifact_path) + if iwc_grade: + lint_context.lint("lint_dockstore_best_practices", _lint_dockstore_config_best_practices, potential_workflow_artifact_path) elif looks_like_a_workflow(potential_workflow_artifact_path): @@ -160,7 +161,7 @@ def structure(path, lint_context): lint_func(lint_context, workflow_dict, path=path) lint_context.lint("lint_structure", structure, potential_workflow_artifact_path) - if lint_context.iwc_grade: + if iwc_grade: lint_context.lint("lint_release", _lint_release, potential_workflow_artifact_path) lint_context.lint("lint_best_practices", _lint_best_practices, potential_workflow_artifact_path) lint_context.lint("lint_tests", _lint_tsts, potential_workflow_artifact_path) @@ -443,14 +444,6 @@ def _lint_dockstore_workflow_entry( lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing required key {required_key}") found_errors = True - if lint_context.iwc_grade: - lint_fun = lint_context.error - else: - lint_fun = lint_context.warn - for recommended_key in ["testParameterFiles", "name"]: - if recommended_key not in workflow_entry: - lint_fun(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}") - if found_errors: # Don't do the rest of the validation for a broken file. return @@ -473,10 +466,6 @@ def _lint_dockstore_workflow_entry( lint_context.error("Dockstore does not accept workflow names with space.", f"Change '{workflow_name}' in {DOCKSTORE_REGISTRY_CONF}.") - # Check there is at least one author - if len(workflow_entry.get('authors', [])) == 0: - lint_fun(f"Workflow {workflow_name} have no " - "'authors' in the .dockstore.yml.") # Check there is not mailto for author in workflow_entry.get('authors', []): if author.get('email', '').startswith('mailto:'): @@ -629,3 +618,36 @@ def _lint_release(path, lint_context): if version != "" and workflow_dict.get("release") != version: lint_context.error(f"The release of workflow {path} does not match " "the version in the CHANGELOG.") + + +def _lint_dockstore_config_best_practices(path: str, lint_context: WorkflowLintContext) -> None: + dockstore_yaml = None + try: + with open(path) as f: + dockstore_yaml = yaml.safe_load(f) + except Exception: + return + + if not isinstance(dockstore_yaml, dict): + return + + workflow_entries = dockstore_yaml.get("workflows") + if not isinstance(workflow_entries, list): + return + + for workflow_entry in workflow_entries: + _lint_dockstore_workflow_entry_best_practices(lint_context, os.path.dirname(path), workflow_entry) + + +def _lint_dockstore_workflow_entry_best_practices( + lint_context: WorkflowLintContext, directory: str, workflow_entry: Dict[str, Any] +) -> None: + for recommended_key in ["testParameterFiles", "name"]: + if recommended_key not in workflow_entry: + lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}") + + workflow_name = workflow_entry.get("name", "") + # Check there is at least one author + if len(workflow_entry.get('authors', [])) == 0: + lint_context.error(f"Workflow {workflow_name} have no " + f"'authors' in the {DOCKSTORE_REGISTRY_CONF}.") From 1310eb72d24bda774eb3b7a1593f0f4826ede7ec Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 13:07:03 +0200 Subject: [PATCH 13/28] new build_wf_lint_args to match what is happening with tools --- planemo/commands/cmd_workflow_lint.py | 10 +++++----- planemo/workflow_lint.py | 7 +++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 5cdf0c4e1..cb8b6adcb 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -4,9 +4,10 @@ from planemo import options from planemo.cli import command_function -from planemo.lint import build_lint_args -from planemo.workflow_lint import lint_workflow_artifacts_on_paths - +from planemo.workflow_lint import ( + build_wf_lint_args, + lint_workflow_artifacts_on_paths +) @click.command("workflow_lint") @options.optional_tools_or_packages_arg(multiple=True) @@ -24,8 +25,7 @@ def cli(ctx, paths, **kwds): """Check workflows for syntax errors and best practices.""" # Unlike tools, lets just make this recursive by default. - lint_args = build_lint_args(ctx, **kwds) - lint_args["iwc_grade"] = str(kwds.get("iwc", False)) + lint_args = build_wf_lint_args(ctx, **kwds) exit_code = lint_workflow_artifacts_on_paths( ctx, paths, diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 389457199..61b0a809b 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -39,6 +39,7 @@ output_labels, required_input_labels, ) +from planemo.lint import build_lint_args from planemo.runnable import ( cases, for_path, @@ -59,6 +60,12 @@ class WorkflowLintContext(LintContext): training_topic = None +def build_wf_lint_args(ctx, **kwds): + lint_args = build_lint_args(ctx, **kwds) + lint_args["iwc_grade"] = str(kwds.get("iwc", False)) + return lint_args + + def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: workflows = [] all_workflow_paths = list(find_workflow_descriptions(directory)) From 906c5cac6d8d5c595a09e10b52c97d55c028087c Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 15:35:33 +0200 Subject: [PATCH 14/28] fix error on CHANGELOG get version --- planemo/workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 61b0a809b..39812aeec 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -621,7 +621,7 @@ def _lint_release(path, lint_context): lint_context.error(f"The workflow {path} has no release") else: # Try to get the version from the CHANGELOG: - version = _get_changelog_version(os.path.join(os.path.dirname(path), "CHANGELOG.md")) + version = _get_changelog_version(os.path.dirname(path)) if version != "" and workflow_dict.get("release") != version: lint_context.error(f"The release of workflow {path} does not match " "the version in the CHANGELOG.") From 39480a8664ba24207ada5e3d590f4e879e25a048 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 15:44:16 +0200 Subject: [PATCH 15/28] add tests --- .../.dockstore.yml | 11 +++ .../basic_wf_iwc_invalid_version/CHANGELOG.md | 5 + .../basic_wf_iwc_invalid_version/README.md | 3 + .../Super-simple-workflow-tests.yml | 10 ++ .../Super-simple-workflow.ga | 95 +++++++++++++++++++ tests/test_cmd_workflow_lint.py | 38 ++++++++ 6 files changed, 162 insertions(+) create mode 100644 tests/data/wf_repos/basic_wf_iwc_invalid_version/.dockstore.yml create mode 100644 tests/data/wf_repos/basic_wf_iwc_invalid_version/CHANGELOG.md create mode 100644 tests/data/wf_repos/basic_wf_iwc_invalid_version/README.md create mode 100644 tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow-tests.yml create mode 100644 tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow.ga diff --git a/tests/data/wf_repos/basic_wf_iwc_invalid_version/.dockstore.yml b/tests/data/wf_repos/basic_wf_iwc_invalid_version/.dockstore.yml new file mode 100644 index 000000000..50c716fee --- /dev/null +++ b/tests/data/wf_repos/basic_wf_iwc_invalid_version/.dockstore.yml @@ -0,0 +1,11 @@ +version: 1.2 +workflows: +- name: main + subclass: Galaxy + publish: true + primaryDescriptorPath: /Super-simple-workflow.ga + testParameterFiles: + - /Super-simple-workflow-tests.yml + authors: + - name: Lucille Delisle + orcid: 0000-0002-1964-4960 diff --git a/tests/data/wf_repos/basic_wf_iwc_invalid_version/CHANGELOG.md b/tests/data/wf_repos/basic_wf_iwc_invalid_version/CHANGELOG.md new file mode 100644 index 000000000..7a859c1bf --- /dev/null +++ b/tests/data/wf_repos/basic_wf_iwc_invalid_version/CHANGELOG.md @@ -0,0 +1,5 @@ +# Changelog + +## [0.1] 2024-06-17 + +First release. diff --git a/tests/data/wf_repos/basic_wf_iwc_invalid_version/README.md b/tests/data/wf_repos/basic_wf_iwc_invalid_version/README.md new file mode 100644 index 000000000..a26db8ed6 --- /dev/null +++ b/tests/data/wf_repos/basic_wf_iwc_invalid_version/README.md @@ -0,0 +1,3 @@ +# Super simple workflow + +This is a super simple workflow which generates a file with x lines with 'hello' diff --git a/tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow-tests.yml b/tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow-tests.yml new file mode 100644 index 000000000..9b1faff97 --- /dev/null +++ b/tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow-tests.yml @@ -0,0 +1,10 @@ +- doc: Test outline for Super-simple-workflow + job: + n_rows: 5 + outputs: + outfile: + asserts: + has_n_lines: + n: 5 + has_line: + line: "hello" diff --git a/tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow.ga b/tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow.ga new file mode 100644 index 000000000..fcc756bd6 --- /dev/null +++ b/tests/data/wf_repos/basic_wf_iwc_invalid_version/Super-simple-workflow.ga @@ -0,0 +1,95 @@ +{ + "a_galaxy_workflow": "true", + "annotation": "This workflow generates a file with x lines with 'hello'", + "comments": [], + "creator": [ + { + "class": "Person", + "identifier": "https://orcid.org/0000-0002-1964-4960", + "name": "Lucille Delisle" + } + ], + "format-version": "0.1", + "license": "MIT", + "release": "0.2", + "name": "Super simple workflow", + "report": { + "markdown": "\n# Workflow Execution Report\n\n## Workflow Inputs\n```galaxy\ninvocation_inputs()\n```\n\n## Workflow Outputs\n```galaxy\ninvocation_outputs()\n```\n\n## Workflow\n```galaxy\nworkflow_display()\n```\n" + }, + "steps": { + "0": { + "annotation": "Number of rows to generate", + "content_id": null, + "errors": null, + "id": 0, + "input_connections": {}, + "inputs": [ + { + "description": "Number of rows to generate", + "name": "n_rows" + } + ], + "label": "n_rows", + "name": "Input parameter", + "outputs": [], + "position": { + "left": 0, + "top": 0 + }, + "tool_id": null, + "tool_state": "{\"parameter_type\": \"integer\", \"optional\": false}", + "tool_version": null, + "type": "parameter_input", + "uuid": "2ee42c13-83f5-4ac4-b35d-74294edf7dea", + "when": null + }, + "1": { + "annotation": "this creates a file with a given number of lines", + "content_id": "toolshed.g2.bx.psu.edu/repos/bgruening/text_processing/tp_text_file_with_recurring_lines/9.3+galaxy1", + "errors": null, + "id": 1, + "input_connections": { + "token_set_0|repeat_select|times": { + "id": 0, + "output_name": "output" + } + }, + "inputs": [], + "label": "create file", + "name": "Create text file", + "outputs": [ + { + "name": "outfile", + "type": "txt" + } + ], + "position": { + "left": 236, + "top": 11 + }, + "post_job_actions": {}, + "tool_id": "toolshed.g2.bx.psu.edu/repos/bgruening/text_processing/tp_text_file_with_recurring_lines/9.3+galaxy1", + "tool_shed_repository": { + "changeset_revision": "fbf99087e067", + "name": "text_processing", + "owner": "bgruening", + "tool_shed": "toolshed.g2.bx.psu.edu" + }, + "tool_state": "{\"token_set\": [{\"__index__\": 0, \"line\": \"hello\", \"repeat_select\": {\"repeat_select_opts\": \"user\", \"__current_case__\": 0, \"times\": {\"__class__\": \"ConnectedValue\"}}}], \"__page__\": null, \"__rerun_remap_job_id__\": null}", + "tool_version": "9.3+galaxy1", + "type": "tool", + "uuid": "a86ff433-8dce-417c-baf0-bc106a93ee48", + "when": null, + "workflow_outputs": [ + { + "label": "outfile", + "output_name": "outfile", + "uuid": "34572088-8ad8-4661-81a2-6dfe2d83a0fe" + } + ] + } + }, + "tags": [], + "uuid": "72c87042-27a8-4bcc-92af-ca74704e6161", + "version": 3 +} \ No newline at end of file diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index 02d38c2e7..d717dafdd 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -226,6 +226,44 @@ def test_workflow_linting_asserts(self): result = self._runner.invoke(self._cli.planemo, lint_cmd) assert "ERROR: Invalid assertion in tests: assert_has_line missing a required argument: 'line'" in result.output + def test_workflow_linting_iwc(self): + # Check the output of workflow_lint --iwc on a basic workflow with .dockstore + repo = _wf_repo("basic_format2_dockstore") + lint_cmd = ["workflow_lint", "--skip", "best_practices", "--iwc", repo] + result = self._runner.invoke(self._cli.planemo, lint_cmd) + + errors = [ + "The file README.md is missing but required.", + "The file CHANGELOG.md is missing but required.", + ".dockstore.yml workflow entry missing recommended key name", + "Workflow have no 'authors' in the .dockstore.yml.", + "The workflow tests/data/wf_repos/basic_format2_dockstore/basic_format2.gxwf.yml has no release" + ] + + for error in errors: + assert error in result.output + + # Check that skipping the good steps makes it work + lint_cmd = ["workflow_lint", "--iwc", "--skip", "best_practices,required_files,dockstore_best_practices,release", repo] + self._check_exit_code(lint_cmd, exit_code=0) + + # Check the output of workflow_lint --iwc on a good workflow but with an issue with the release + repo = _wf_repo("basic_wf_iwc_invalid_version") + lint_cmd = ["workflow_lint", "--iwc", repo] + result = self._runner.invoke(self._cli.planemo, lint_cmd) + + errors = [ + "The release of workflow", + " does not match the version in the CHANGELOG." + ] + + for error in errors: + assert error in result.output + + # Check that skipping the good steps makes it work + lint_cmd = ["workflow_lint", "--iwc", "--skip", "release", repo] + self._check_exit_code(lint_cmd, exit_code=0) + def _wf_repo(rel_path): return os.path.join(TEST_DATA_DIR, "wf_repos", rel_path) From 7bb3a50b4b8732a397982d5243edb1d8f2a4675a Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 15:46:44 +0200 Subject: [PATCH 16/28] restore comma --- planemo/commands/cmd_workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index cb8b6adcb..dc94c03d7 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -29,6 +29,6 @@ def cli(ctx, paths, **kwds): exit_code = lint_workflow_artifacts_on_paths( ctx, paths, - lint_args + lint_args, ) ctx.exit(exit_code) From db8f24cc5eb9d9f3383b2071e94669a41abf50b9 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 15:49:11 +0200 Subject: [PATCH 17/28] test --iwc on file --- tests/test_cmd_workflow_lint.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index d717dafdd..be9615a7b 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -264,6 +264,17 @@ def test_workflow_linting_iwc(self): lint_cmd = ["workflow_lint", "--iwc", "--skip", "release", repo] self._check_exit_code(lint_cmd, exit_code=0) + # Check the output of workflow_lint --iwc on a file raise an error + repo = _wf_repo("wf1.ga") + lint_cmd = ["workflow_lint", "--iwc", repo] + result = self._runner.invoke(self._cli.planemo, lint_cmd) + + errors = [ + "ValueError: iwc standards can only be checked on directories." + ] + + for error in errors: + assert error in result.output def _wf_repo(rel_path): return os.path.join(TEST_DATA_DIR, "wf_repos", rel_path) From 22112e88f98da759c75bb9895007c2edf44da50b Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 16:20:07 +0200 Subject: [PATCH 18/28] remove path from asserts output --- tests/test_cmd_workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index be9615a7b..1c872f525 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -237,7 +237,7 @@ def test_workflow_linting_iwc(self): "The file CHANGELOG.md is missing but required.", ".dockstore.yml workflow entry missing recommended key name", "Workflow have no 'authors' in the .dockstore.yml.", - "The workflow tests/data/wf_repos/basic_format2_dockstore/basic_format2.gxwf.yml has no release" + "has no release" ] for error in errors: From 147296ae1cb3e31e2139eb28aa2ae3e7fbfb6997 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 17:38:45 +0200 Subject: [PATCH 19/28] run black --- planemo/commands/cmd_workflow_lint.py | 6 +-- planemo/workflow_lint.py | 57 ++++++++++++++------------- tests/test_cmd_workflow_lint.py | 24 +++++------ 3 files changed, 45 insertions(+), 42 deletions(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index dc94c03d7..2508c6236 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -4,10 +4,8 @@ from planemo import options from planemo.cli import command_function -from planemo.workflow_lint import ( - build_wf_lint_args, - lint_workflow_artifacts_on_paths -) +from planemo.workflow_lint import build_wf_lint_args, lint_workflow_artifacts_on_paths + @click.command("workflow_lint") @options.optional_tools_or_packages_arg(multiple=True) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 39812aeec..190221277 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -156,7 +156,11 @@ def _lint_workflow_artifacts_on_path( if os.path.basename(potential_workflow_artifact_path) == DOCKSTORE_REGISTRY_CONF: lint_context.lint("lint_dockstore", _lint_dockstore_config, potential_workflow_artifact_path) if iwc_grade: - lint_context.lint("lint_dockstore_best_practices", _lint_dockstore_config_best_practices, potential_workflow_artifact_path) + lint_context.lint( + "lint_dockstore_best_practices", + _lint_dockstore_config_best_practices, + potential_workflow_artifact_path, + ) elif looks_like_a_workflow(potential_workflow_artifact_path): @@ -469,15 +473,16 @@ def _lint_dockstore_workflow_entry( # Check there is no space in name: workflow_name = workflow_entry.get("name", "") # Check the name has no space - if ' ' in workflow_name: - lint_context.error("Dockstore does not accept workflow names with space.", - f"Change '{workflow_name}' in {DOCKSTORE_REGISTRY_CONF}.") + if " " in workflow_name: + lint_context.error( + "Dockstore does not accept workflow names with space.", + f"Change '{workflow_name}' in {DOCKSTORE_REGISTRY_CONF}.", + ) # Check there is not mailto - for author in workflow_entry.get('authors', []): - if author.get('email', '').startswith('mailto:'): - lint_context.error("email field of the .dockstore.yml must not " - "contain 'mailto:'") + for author in workflow_entry.get("authors", []): + if author.get("email", "").startswith("mailto:"): + lint_context.error("email field of the .dockstore.yml must not " "contain 'mailto:'") def looks_like_a_workflow(path: str) -> bool: @@ -583,48 +588,47 @@ def _lint_tool_ids_steps(lint_context: WorkflowLintContext, wf_dict: Dict, ts: T def _lint_required_files_workflow_dir(path: str, lint_context: WorkflowLintContext) -> None: # Check all required files are present - required_files = \ - ['README.md', 'CHANGELOG.md', '.dockstore.yml'] + required_files = ["README.md", "CHANGELOG.md", ".dockstore.yml"] for required_file in required_files: if not os.path.exists(os.path.join(path, required_file)): - lint_context.error(f"The file {required_file} is" - " missing but required.") + lint_context.error(f"The file {required_file} is" " missing but required.") def _get_changelog_version(path: str) -> str: # Get the version from the CHANGELOG.md version = "" - if not os.path.exists(os.path.join(path, 'CHANGELOG.md')): + if not os.path.exists(os.path.join(path, "CHANGELOG.md")): return version - with open(os.path.join(path, 'CHANGELOG.md'), 'r') as f: + with open(os.path.join(path, "CHANGELOG.md"), "r") as f: for line in f: - if line.startswith('## ['): - version = line.split(']')[0].replace('## [', '') + if line.startswith("## ["): + version = line.split("]")[0].replace("## [", "") break return version def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> None: # Check the version can be get from the CHANGELOG.md - if not os.path.exists(os.path.join(path, 'CHANGELOG.md')): + if not os.path.exists(os.path.join(path, "CHANGELOG.md")): return if _get_changelog_version(path) == "": - lint_context.error("No version found in CHANGELOG. " - "The version should be in a line that starts like " - "'## [version number]'") + lint_context.error( + "No version found in CHANGELOG. " + "The version should be in a line that starts like " + "'## [version number]'" + ) def _lint_release(path, lint_context): with open(path) as f: workflow_dict = ordered_load(f) - if 'release' not in workflow_dict: + if "release" not in workflow_dict: lint_context.error(f"The workflow {path} has no release") else: # Try to get the version from the CHANGELOG: version = _get_changelog_version(os.path.dirname(path)) if version != "" and workflow_dict.get("release") != version: - lint_context.error(f"The release of workflow {path} does not match " - "the version in the CHANGELOG.") + lint_context.error(f"The release of workflow {path} does not match " "the version in the CHANGELOG.") def _lint_dockstore_config_best_practices(path: str, lint_context: WorkflowLintContext) -> None: @@ -634,7 +638,7 @@ def _lint_dockstore_config_best_practices(path: str, lint_context: WorkflowLintC dockstore_yaml = yaml.safe_load(f) except Exception: return - + if not isinstance(dockstore_yaml, dict): return @@ -655,6 +659,5 @@ def _lint_dockstore_workflow_entry_best_practices( workflow_name = workflow_entry.get("name", "") # Check there is at least one author - if len(workflow_entry.get('authors', [])) == 0: - lint_context.error(f"Workflow {workflow_name} have no " - f"'authors' in the {DOCKSTORE_REGISTRY_CONF}.") + if len(workflow_entry.get("authors", [])) == 0: + lint_context.error(f"Workflow {workflow_name} have no 'authors' in the {DOCKSTORE_REGISTRY_CONF}.") diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index 1c872f525..ea26e53a8 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -237,14 +237,20 @@ def test_workflow_linting_iwc(self): "The file CHANGELOG.md is missing but required.", ".dockstore.yml workflow entry missing recommended key name", "Workflow have no 'authors' in the .dockstore.yml.", - "has no release" + "has no release", ] for error in errors: assert error in result.output - # Check that skipping the good steps makes it work - lint_cmd = ["workflow_lint", "--iwc", "--skip", "best_practices,required_files,dockstore_best_practices,release", repo] + # Check that skipping the good steps makes it work + lint_cmd = [ + "workflow_lint", + "--iwc", + "--skip", + "best_practices,required_files,dockstore_best_practices,release", + repo, + ] self._check_exit_code(lint_cmd, exit_code=0) # Check the output of workflow_lint --iwc on a good workflow but with an issue with the release @@ -252,15 +258,12 @@ def test_workflow_linting_iwc(self): lint_cmd = ["workflow_lint", "--iwc", repo] result = self._runner.invoke(self._cli.planemo, lint_cmd) - errors = [ - "The release of workflow", - " does not match the version in the CHANGELOG." - ] + errors = ["The release of workflow", " does not match the version in the CHANGELOG."] for error in errors: assert error in result.output - # Check that skipping the good steps makes it work + # Check that skipping the good steps makes it work lint_cmd = ["workflow_lint", "--iwc", "--skip", "release", repo] self._check_exit_code(lint_cmd, exit_code=0) @@ -269,12 +272,11 @@ def test_workflow_linting_iwc(self): lint_cmd = ["workflow_lint", "--iwc", repo] result = self._runner.invoke(self._cli.planemo, lint_cmd) - errors = [ - "ValueError: iwc standards can only be checked on directories." - ] + errors = ["ValueError: iwc standards can only be checked on directories."] for error in errors: assert error in result.output + def _wf_repo(rel_path): return os.path.join(TEST_DATA_DIR, "wf_repos", rel_path) From f5af20fe79c3869a706686efa9344a5b8ed67eb2 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 17:45:15 +0200 Subject: [PATCH 20/28] run isort --- planemo/commands/cmd_workflow_lint.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 2508c6236..91bcacfa3 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -4,7 +4,10 @@ from planemo import options from planemo.cli import command_function -from planemo.workflow_lint import build_wf_lint_args, lint_workflow_artifacts_on_paths +from planemo.workflow_lint import ( + build_wf_lint_args, + lint_workflow_artifacts_on_paths, +) @click.command("workflow_lint") From 6e8f0ce67e278c6a0fff46f871bbfd9273cd6697 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 18:04:07 +0200 Subject: [PATCH 21/28] fix strange output when no --skip --- planemo/lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/lint.py b/planemo/lint.py index 5b85022de..c5964428a 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -23,7 +23,7 @@ def build_lint_args(ctx, **kwds): skip = ctx.global_config.get("lint_skip", "") if isinstance(skip, list): skip = ",".join(skip) - skip_types = [s.strip() for s in skip.split(",")] + skip_types = [s.strip() for s in skip.split(",") if s.strip()] for skip_file in kwds.get("skip_file", []): with open(skip_file) as f: From f748c11dfe9a1326772d7aabce948d0eee4dee16 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 18:14:38 +0200 Subject: [PATCH 22/28] fix final test --- tests/test_cmd_workflow_lint.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index ea26e53a8..2a5920bf8 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -271,11 +271,9 @@ def test_workflow_linting_iwc(self): repo = _wf_repo("wf1.ga") lint_cmd = ["workflow_lint", "--iwc", repo] result = self._runner.invoke(self._cli.planemo, lint_cmd) - - errors = ["ValueError: iwc standards can only be checked on directories."] - - for error in errors: - assert error in result.output + assert result.exit_code == 1 + assert isinstance(result.exception, ValueError) + assert str(result.exception) == "iwc standards can only be checked on directories." def _wf_repo(rel_path): From 0d8dbd6c67f30a2248b18199b7a0c67d2246d6d3 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 22:50:41 +0200 Subject: [PATCH 23/28] change strategy for iwc on files --- planemo/workflow_lint.py | 2 +- tests/test_cmd_workflow_lint.py | 39 ++++++++++++++------------------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 190221277..0a2cbda04 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -148,7 +148,7 @@ def _lint_workflow_artifacts_on_path( iwc_grade = lint_args["iwc_grade"] == "True" if iwc_grade: if not os.path.isdir(path): - raise ValueError("iwc standards can only be checked on directories.") + path = os.path.dirname(path) lint_context.lint("lint_required_files", _lint_required_files_workflow_dir, path) lint_context.lint("lint_changelog", _lint_changelog_version, path) diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index 2a5920bf8..c3155489e 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -228,20 +228,23 @@ def test_workflow_linting_asserts(self): def test_workflow_linting_iwc(self): # Check the output of workflow_lint --iwc on a basic workflow with .dockstore - repo = _wf_repo("basic_format2_dockstore") - lint_cmd = ["workflow_lint", "--skip", "best_practices", "--iwc", repo] - result = self._runner.invoke(self._cli.planemo, lint_cmd) - - errors = [ - "The file README.md is missing but required.", - "The file CHANGELOG.md is missing but required.", - ".dockstore.yml workflow entry missing recommended key name", - "Workflow have no 'authors' in the .dockstore.yml.", - "has no release", - ] - - for error in errors: - assert error in result.output + for repo in [ + _wf_repo("basic_format2_dockstore"), + _wf_repo(os.path.join("basic_format2_dockstore", "basic_format2.gxwf.yml")) + ]: + lint_cmd = ["workflow_lint", "--skip", "best_practices", "--iwc", repo] + result = self._runner.invoke(self._cli.planemo, lint_cmd) + + errors = [ + "The file README.md is missing but required.", + "The file CHANGELOG.md is missing but required.", + ".dockstore.yml workflow entry missing recommended key name", + "Workflow have no 'authors' in the .dockstore.yml.", + "has no release", + ] + + for error in errors: + assert error in result.output # Check that skipping the good steps makes it work lint_cmd = [ @@ -267,14 +270,6 @@ def test_workflow_linting_iwc(self): lint_cmd = ["workflow_lint", "--iwc", "--skip", "release", repo] self._check_exit_code(lint_cmd, exit_code=0) - # Check the output of workflow_lint --iwc on a file raise an error - repo = _wf_repo("wf1.ga") - lint_cmd = ["workflow_lint", "--iwc", repo] - result = self._runner.invoke(self._cli.planemo, lint_cmd) - assert result.exit_code == 1 - assert isinstance(result.exception, ValueError) - assert str(result.exception) == "iwc standards can only be checked on directories." - def _wf_repo(rel_path): return os.path.join(TEST_DATA_DIR, "wf_repos", rel_path) From 64f90768bb243a37feb4b3bce5f235af078cd6e6 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Mon, 1 Jul 2024 23:04:58 +0200 Subject: [PATCH 24/28] add comma --- tests/test_cmd_workflow_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cmd_workflow_lint.py b/tests/test_cmd_workflow_lint.py index c3155489e..9377464f5 100644 --- a/tests/test_cmd_workflow_lint.py +++ b/tests/test_cmd_workflow_lint.py @@ -230,7 +230,7 @@ def test_workflow_linting_iwc(self): # Check the output of workflow_lint --iwc on a basic workflow with .dockstore for repo in [ _wf_repo("basic_format2_dockstore"), - _wf_repo(os.path.join("basic_format2_dockstore", "basic_format2.gxwf.yml")) + _wf_repo(os.path.join("basic_format2_dockstore", "basic_format2.gxwf.yml")), ]: lint_cmd = ["workflow_lint", "--skip", "best_practices", "--iwc", repo] result = self._runner.invoke(self._cli.planemo, lint_cmd) From bafecffb2f1e89a0bb8dd309481495dce305bed3 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Tue, 2 Jul 2024 12:15:32 +0100 Subject: [PATCH 25/28] Add type annotation --- planemo/commands/cmd_lint.py | 7 +++++-- planemo/commands/cmd_shed_lint.py | 7 +++++-- planemo/commands/cmd_workflow_lint.py | 7 +++++-- planemo/lint.py | 12 ++++++++++-- planemo/shed_lint.py | 6 +++++- planemo/tool_lint.py | 6 +++++- planemo/workflow_lint.py | 4 ++-- 7 files changed, 37 insertions(+), 12 deletions(-) diff --git a/planemo/commands/cmd_lint.py b/planemo/commands/cmd_lint.py index 85a3c77cc..f6ca5a454 100644 --- a/planemo/commands/cmd_lint.py +++ b/planemo/commands/cmd_lint.py @@ -3,7 +3,10 @@ import click from planemo import options -from planemo.cli import command_function +from planemo.cli import ( + command_function, + PlanemoCliContext, +) from planemo.tool_lint import ( build_tool_lint_args, lint_tools_on_path, @@ -43,7 +46,7 @@ # default=False, # ) @command_function -def cli(ctx, uris, **kwds): +def cli(ctx: PlanemoCliContext, uris, **kwds): """Check for common errors and best practices.""" lint_args = build_tool_lint_args(ctx, **kwds) exit_code = lint_tools_on_path(ctx, uris, lint_args, recursive=kwds["recursive"]) diff --git a/planemo/commands/cmd_shed_lint.py b/planemo/commands/cmd_shed_lint.py index 80f420b31..70bb5c9d4 100644 --- a/planemo/commands/cmd_shed_lint.py +++ b/planemo/commands/cmd_shed_lint.py @@ -7,7 +7,10 @@ shed, shed_lint, ) -from planemo.cli import command_function +from planemo.cli import ( + command_function, + PlanemoCliContext, +) @click.command("shed_lint") @@ -41,7 +44,7 @@ # default=False, # ) @command_function -def cli(ctx, paths, **kwds): +def cli(ctx: PlanemoCliContext, paths, **kwds): """Check Tool Shed repository for common issues. With the ``--tools`` flag, this command lints actual Galaxy tools diff --git a/planemo/commands/cmd_workflow_lint.py b/planemo/commands/cmd_workflow_lint.py index 91bcacfa3..e71fdd61f 100644 --- a/planemo/commands/cmd_workflow_lint.py +++ b/planemo/commands/cmd_workflow_lint.py @@ -3,7 +3,10 @@ import click from planemo import options -from planemo.cli import command_function +from planemo.cli import ( + command_function, + PlanemoCliContext, +) from planemo.workflow_lint import ( build_wf_lint_args, lint_workflow_artifacts_on_paths, @@ -23,7 +26,7 @@ help="Check workflows directory with the standards of iwc", ) @command_function -def cli(ctx, paths, **kwds): +def cli(ctx: PlanemoCliContext, paths, **kwds): """Check workflows for syntax errors and best practices.""" # Unlike tools, lets just make this recursive by default. lint_args = build_wf_lint_args(ctx, **kwds) diff --git a/planemo/lint.py b/planemo/lint.py index c14247e7a..87bfd5aaa 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -1,6 +1,11 @@ """Utilities to help linting various targets.""" import os +from typing import ( + Any, + Dict, + TYPE_CHECKING, +) from urllib.request import urlopen import requests @@ -13,8 +18,11 @@ from planemo.shed import find_urls_for_xml from planemo.xml import validation +if TYPE_CHECKING: + from planemo.cli import PlanemoCliContext + -def build_lint_args(ctx, **kwds): +def build_lint_args(ctx: "PlanemoCliContext", **kwds) -> Dict[str, Any]: """Handle common report, error, and skip linting arguments.""" report_level = kwds.get("report_level", "all") fail_level = kwds.get("fail_level", "warn") @@ -39,7 +47,7 @@ def build_lint_args(ctx, **kwds): if len(invalid_skip_types): error(f"Unknown linter type(s) {invalid_skip_types} in list of linters to be skipped. Known linters {linters}") - lint_args = dict( + lint_args: Dict[str, Any] = dict( level=report_level, fail_level=fail_level, skip_types=skip_types, diff --git a/planemo/shed_lint.py b/planemo/shed_lint.py index 322d16756..b3d03760b 100644 --- a/planemo/shed_lint.py +++ b/planemo/shed_lint.py @@ -2,6 +2,7 @@ import os import xml.etree.ElementTree as ET +from typing import TYPE_CHECKING import yaml from galaxy.tool_util.lint import lint_tool_source_with @@ -31,6 +32,9 @@ from planemo.tools import yield_tool_sources from planemo.xml import XSDS_PATH +if TYPE_CHECKING: + from planemo.cli import PlanemoCliContext + TOOL_DEPENDENCIES_XSD = os.path.join(XSDS_PATH, "tool_dependencies.xsd") REPO_DEPENDENCIES_XSD = os.path.join(XSDS_PATH, "repository_dependencies.xsd") @@ -50,7 +54,7 @@ ] -def lint_repository(ctx, realized_repository, **kwds): +def lint_repository(ctx: "PlanemoCliContext", realized_repository, **kwds): """Lint a realized shed repository. See :mod:`planemo.shed` for details on constructing a realized diff --git a/planemo/tool_lint.py b/planemo/tool_lint.py index 9e72d2646..d6ccd509e 100644 --- a/planemo/tool_lint.py +++ b/planemo/tool_lint.py @@ -1,4 +1,5 @@ from os.path import basename +from typing import TYPE_CHECKING from galaxy.tool_util.lint import lint_tool_source @@ -22,10 +23,13 @@ yield_tool_sources_on_paths, ) +if TYPE_CHECKING: + from planemo.cli import PlanemoCliContext + LINTING_TOOL_MESSAGE = "Linting tool %s" -def build_tool_lint_args(ctx, **kwds): +def build_tool_lint_args(ctx: "PlanemoCliContext", **kwds): lint_args = build_lint_args(ctx, **kwds) extra_modules = _lint_extra_modules(**kwds) lint_args["extra_modules"] = extra_modules diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 0a2cbda04..b79c8ca4b 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -60,9 +60,9 @@ class WorkflowLintContext(LintContext): training_topic = None -def build_wf_lint_args(ctx, **kwds): +def build_wf_lint_args(ctx: "PlanemoCliContext", **kwds) -> Dict[str, Any]: lint_args = build_lint_args(ctx, **kwds) - lint_args["iwc_grade"] = str(kwds.get("iwc", False)) + lint_args["iwc_grade"] = kwds.get("iwc", False) return lint_args From b1eedf4ef70fab31d28a8280edab4a1eaa28e6ca Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Tue, 2 Jul 2024 13:21:22 +0200 Subject: [PATCH 26/28] Apply suggestions from @bernt-matthias Co-authored-by: M Bernt --- planemo/workflow_lint.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index b79c8ca4b..c1f9df8c8 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -145,8 +145,7 @@ def lint_workflow_artifacts_on_paths( def _lint_workflow_artifacts_on_path( lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Union[str, List[str]]] ) -> None: - iwc_grade = lint_args["iwc_grade"] == "True" - if iwc_grade: + if lint_args["iwc_grade"]: if not os.path.isdir(path): path = os.path.dirname(path) lint_context.lint("lint_required_files", _lint_required_files_workflow_dir, path) @@ -155,7 +154,7 @@ def _lint_workflow_artifacts_on_path( for potential_workflow_artifact_path in find_potential_workflow_files(path): if os.path.basename(potential_workflow_artifact_path) == DOCKSTORE_REGISTRY_CONF: lint_context.lint("lint_dockstore", _lint_dockstore_config, potential_workflow_artifact_path) - if iwc_grade: + if lint_args["iwc_grade"]: lint_context.lint( "lint_dockstore_best_practices", _lint_dockstore_config_best_practices, @@ -172,7 +171,7 @@ def structure(path, lint_context): lint_func(lint_context, workflow_dict, path=path) lint_context.lint("lint_structure", structure, potential_workflow_artifact_path) - if iwc_grade: + if lint_args["iwc_grade"]: lint_context.lint("lint_release", _lint_release, potential_workflow_artifact_path) lint_context.lint("lint_best_practices", _lint_best_practices, potential_workflow_artifact_path) lint_context.lint("lint_tests", _lint_tsts, potential_workflow_artifact_path) From ecdcf0f80c473a4e8ca5748470151ee7c68a5199 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Tue, 2 Jul 2024 13:30:22 +0200 Subject: [PATCH 27/28] fix lint_args typing to different places --- planemo/tool_lint.py | 8 ++++++-- planemo/workflow_lint.py | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/planemo/tool_lint.py b/planemo/tool_lint.py index d6ccd509e..eea2ad7ff 100644 --- a/planemo/tool_lint.py +++ b/planemo/tool_lint.py @@ -1,5 +1,9 @@ from os.path import basename -from typing import TYPE_CHECKING +from typing import ( + Any, + Dict, + TYPE_CHECKING, +) from galaxy.tool_util.lint import lint_tool_source @@ -29,7 +33,7 @@ LINTING_TOOL_MESSAGE = "Linting tool %s" -def build_tool_lint_args(ctx: "PlanemoCliContext", **kwds): +def build_tool_lint_args(ctx: "PlanemoCliContext", **kwds) -> Dict[str, Any]: lint_args = build_lint_args(ctx, **kwds) extra_modules = _lint_extra_modules(**kwds) lint_args["extra_modules"] = extra_modules diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index c1f9df8c8..085a25fd7 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -129,7 +129,7 @@ def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: def lint_workflow_artifacts_on_paths( - ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]] + ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Any] ) -> int: report_level = lint_args["level"] lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"]) @@ -143,7 +143,7 @@ def lint_workflow_artifacts_on_paths( def _lint_workflow_artifacts_on_path( - lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Union[str, List[str]]] + lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Any] ) -> None: if lint_args["iwc_grade"]: if not os.path.isdir(path): From 2f7c139cb0cc6e9984bf6b0c29dcd950da2ad5f7 Mon Sep 17 00:00:00 2001 From: Lucille Delisle Date: Tue, 2 Jul 2024 13:35:02 +0200 Subject: [PATCH 28/28] remove unused Union --- planemo/workflow_lint.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/planemo/workflow_lint.py b/planemo/workflow_lint.py index 085a25fd7..b8403f41c 100644 --- a/planemo/workflow_lint.py +++ b/planemo/workflow_lint.py @@ -12,7 +12,6 @@ Optional, Tuple, TYPE_CHECKING, - Union, ) import requests @@ -128,9 +127,7 @@ def generate_dockstore_yaml(directory: str, publish: bool = True) -> str: return contents -def lint_workflow_artifacts_on_paths( - ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Any] -) -> int: +def lint_workflow_artifacts_on_paths(ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Any]) -> int: report_level = lint_args["level"] lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"]) for path in paths: @@ -142,9 +139,7 @@ def lint_workflow_artifacts_on_paths( return EXIT_CODE_OK -def _lint_workflow_artifacts_on_path( - lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Any] -) -> None: +def _lint_workflow_artifacts_on_path(lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Any]) -> None: if lint_args["iwc_grade"]: if not os.path.isdir(path): path = os.path.dirname(path)