From 8f23b170a5bf5d877cd0a31338c20d36bee4abde Mon Sep 17 00:00:00 2001 From: fabianegli Date: Wed, 20 Jul 2022 13:14:16 +0200 Subject: [PATCH 1/4] remove assert and code cleanup --- nf_core/__main__.py | 32 +++---- nf_core/launch.py | 14 +++- nf_core/lint/actions_awsfulltest.py | 9 +- nf_core/lint/actions_awstest.py | 7 +- nf_core/lint/actions_ci.py | 25 ++++-- nf_core/lint/multiqc_config.py | 43 ++++++---- nf_core/lint/nextflow_config.py | 16 ++-- nf_core/lint/readme.py | 6 +- .../bin/check_samplesheet.py | 23 ++--- .../templates/dumpsoftwareversions.py | 11 +-- nf_core/schema.py | 83 +++++++++++-------- nf_core/utils.py | 8 +- 12 files changed, 159 insertions(+), 118 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index e66d9cead2..0c89b608f1 100755 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -919,7 +919,13 @@ def lint(schema_path): @schema.command() -@click.argument("schema_path", type=click.Path(exists=True), required=False, metavar="") +@click.argument( + "schema_path", + type=click.Path(exists=True), + default="nextflow_schema.json", + required=False, + metavar="", +) @click.option("-o", "--output", type=str, metavar="", help="Output filename. Defaults to standard out.") @click.option( "-x", "--format", type=click.Choice(["markdown", "html"]), default="markdown", help="Format to output docs in." @@ -937,23 +943,17 @@ def docs(schema_path, output, format, force, columns): """ Outputs parameter documentation for a pipeline schema. """ - schema_obj = nf_core.schema.PipelineSchema() - try: - # Assume we're in a pipeline dir root if schema path not set - if schema_path is None: - schema_path = "nextflow_schema.json" - assert os.path.exists( - schema_path - ), "Could not find 'nextflow_schema.json' in current directory. Please specify a path." - schema_obj.get_schema_path(schema_path) - schema_obj.load_schema() - docs = schema_obj.print_documentation(output, format, force, columns.split(",")) - if not output: - print(docs) - except AssertionError as e: - log.error(e) + if not os.path.exists(schema_path): + log.error("Could not find 'nextflow_schema.json' in current directory. Please specify a path.") sys.exit(1) + schema_obj = nf_core.schema.PipelineSchema() + # Assume we're in a pipeline dir root if schema path not set + schema_obj.get_schema_path(schema_path) + schema_obj.load_schema() + if not output: + print(schema_obj.print_documentation(output, format, force, columns.split(","))) + # nf-core bump-version @nf_core_cli.command("bump-version") diff --git a/nf_core/launch.py b/nf_core/launch.py index f5f5145aa2..45d4cb3efc 100644 --- a/nf_core/launch.py +++ b/nf_core/launch.py @@ -301,10 +301,15 @@ def launch_web_gui(self): } web_response = nf_core.utils.poll_nfcore_web_api(self.web_schema_launch_url, content) try: - assert "api_url" in web_response - assert "web_url" in web_response + if "api_url" not in web_response: + raise AssertionError('"api_url" not in web_response') + if "web_url" not in web_response: + raise AssertionError('"web_url" not in web_response') # DO NOT FIX THIS TYPO. Needs to stay in sync with the website. Maintaining for backwards compatability. - assert web_response["status"] == "recieved" + if web_response["status"] != "recieved": + raise AssertionError( + f'web_response["status"] should be "recieved", but it is "{web_response["status"]}"' + ) except AssertionError: log.debug(f"Response content:\n{json.dumps(web_response, indent=4)}") raise AssertionError( @@ -597,7 +602,8 @@ def validate_integer(val): try: if val.strip() == "": return True - assert int(val) == float(val) + if int(val) != float(val): + raise AssertionError(f'Expected an integer, got "{val}"') except (AssertionError, ValueError): return "Must be an integer" else: diff --git a/nf_core/lint/actions_awsfulltest.py b/nf_core/lint/actions_awsfulltest.py index 353004f50c..e021ebd384 100644 --- a/nf_core/lint/actions_awsfulltest.py +++ b/nf_core/lint/actions_awsfulltest.py @@ -43,8 +43,10 @@ def actions_awsfulltest(self): # Check that the action is only turned on for published releases try: - assert wf[True]["release"]["types"] == ["published"] - assert "workflow_dispatch" in wf[True] + if wf[True]["release"]["types"] != ["published"]: + raise AssertionError() + if "workflow_dispatch" not in wf[True]: + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append("`.github/workflows/awsfulltest.yml` is not triggered correctly") else: @@ -53,7 +55,8 @@ def actions_awsfulltest(self): # Warn if `-profile test` is still unchanged try: steps = wf["jobs"]["run-tower"]["steps"] - assert any([aws_profile in step["run"] for step in steps if "run" in step.keys()]) + if not any(aws_profile in step["run"] for step in steps if "run" in step.keys()): + raise AssertionError() except (AssertionError, KeyError, TypeError): passed.append("`.github/workflows/awsfulltest.yml` does not use `-profile test`") else: diff --git a/nf_core/lint/actions_awstest.py b/nf_core/lint/actions_awstest.py index d04fde67c4..4f27cbd765 100644 --- a/nf_core/lint/actions_awstest.py +++ b/nf_core/lint/actions_awstest.py @@ -36,9 +36,10 @@ def actions_awstest(self): # Check that the action is only turned on for workflow_dispatch try: - assert "workflow_dispatch" in wf[True] - assert "push" not in wf[True] - assert "pull_request" not in wf[True] + if "workflow_dispatch" not in wf[True]: + raise AssertionError() + if "push" in wf[True] or "pull_request" in wf[True]: + raise AssertionError() except (AssertionError, KeyError, TypeError): return {"failed": ["'.github/workflows/awstest.yml' is not triggered correctly"]} else: diff --git a/nf_core/lint/actions_ci.py b/nf_core/lint/actions_ci.py index 4d594c8d75..4d6b0e6dfe 100644 --- a/nf_core/lint/actions_ci.py +++ b/nf_core/lint/actions_ci.py @@ -87,14 +87,17 @@ def actions_ci(self): # Check that the action is turned on for the correct events try: # NB: YAML dict key 'on' is evaluated to a Python dict key True - assert "dev" in ciwf[True]["push"]["branches"] + if "dev" not in ciwf[True]["push"]["branches"]: + raise AssertionError() pr_subtree = ciwf[True]["pull_request"] - assert ( - pr_subtree == None + if not ( + pr_subtree is None or ("branches" in pr_subtree and "dev" in pr_subtree["branches"]) or ("ignore_branches" in pr_subtree and not "dev" in pr_subtree["ignore_branches"]) - ) - assert "published" in ciwf[True]["release"]["types"] + ): + raise AssertionError() + if "published" not in ciwf[True]["release"]["types"]: + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append("'.github/workflows/ci.yml' is not triggered on expected events") else: @@ -109,7 +112,8 @@ def actions_ci(self): docker_build_cmd = f"docker build --no-cache . -t {docker_withtag}" try: steps = ciwf["jobs"]["test"]["steps"] - assert any([docker_build_cmd in step["run"] for step in steps if "run" in step.keys()]) + if not any(docker_build_cmd in step["run"] for step in steps if "run" in step.keys()): + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append(f"CI is not building the correct docker image. Should be: `{docker_build_cmd}`") else: @@ -119,7 +123,8 @@ def actions_ci(self): docker_pull_cmd = f"docker pull {docker_notag}:dev" try: steps = ciwf["jobs"]["test"]["steps"] - assert any([docker_pull_cmd in step["run"] for step in steps if "run" in step.keys()]) + if not any(docker_pull_cmd in step["run"] for step in steps if "run" in step.keys()): + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append(f"CI is not pulling the correct docker image. Should be: `{docker_pull_cmd}`") else: @@ -129,7 +134,8 @@ def actions_ci(self): docker_tag_cmd = f"docker tag {docker_notag}:dev {docker_withtag}" try: steps = ciwf["jobs"]["test"]["steps"] - assert any([docker_tag_cmd in step["run"] for step in steps if "run" in step.keys()]) + if not any(docker_tag_cmd in step["run"] for step in steps if "run" in step.keys()): + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append(f"CI is not tagging docker image correctly. Should be: `{docker_tag_cmd}`") else: @@ -138,7 +144,8 @@ def actions_ci(self): # Check that we are testing the minimum nextflow version try: nxf_ver = ciwf["jobs"]["test"]["strategy"]["matrix"]["NXF_VER"] - assert any([i == self.minNextflowVersion for i in nxf_ver]) + if not any(i == self.minNextflowVersion for i in nxf_ver): + raise AssertionError() except (KeyError, TypeError): failed.append("'.github/workflows/ci.yml' does not check minimum NF version") except AssertionError: diff --git a/nf_core/lint/multiqc_config.py b/nf_core/lint/multiqc_config.py index b6bff402ce..30f43bc50b 100644 --- a/nf_core/lint/multiqc_config.py +++ b/nf_core/lint/multiqc_config.py @@ -23,7 +23,6 @@ def multiqc_config(self): export_plots: true """ passed = [] - warned = [] failed = [] # Remove field that should be ignored according to the linting config @@ -43,27 +42,29 @@ def multiqc_config(self): # Check that the report_comment exists and matches try: - assert "report_section_order" in mqc_yml + if "report_section_order" not in mqc_yml: + raise AssertionError() orders = dict() summary_plugin_name = f"{self.pipeline_prefix}-{self.pipeline_name}-summary" min_plugins = ["software_versions", summary_plugin_name] for plugin in min_plugins: - assert plugin in mqc_yml["report_section_order"], f"Section {plugin} missing in report_section_order" - assert "order" in mqc_yml["report_section_order"][plugin], f"Section {plugin} 'order' missing. Must be < 0" + if plugin not in mqc_yml["report_section_order"]: + raise AssertionError(f"Section {plugin} missing in report_section_order") + if "order" not in mqc_yml["report_section_order"][plugin]: + raise AssertionError(f"Section {plugin} 'order' missing. Must be < 0") plugin_order = mqc_yml["report_section_order"][plugin]["order"] - assert plugin_order < 0, f"Section {plugin} 'order' must be < 0" + if plugin_order >= 0: + raise AssertionError(f"Section {plugin} 'order' must be < 0") for plugin in mqc_yml["report_section_order"]: if "order" in mqc_yml["report_section_order"][plugin]: orders[plugin] = mqc_yml["report_section_order"][plugin]["order"] - assert orders[summary_plugin_name] == min( - orders.values() - ), f"Section {summary_plugin_name} should have the lowest order" + if orders[summary_plugin_name] != min(orders.values()): + raise AssertionError(f"Section {summary_plugin_name} should have the lowest order") orders.pop(summary_plugin_name) - assert orders["software_versions"] == min( - orders.values() - ), "Section software_versions should have the second lowest order" + if orders["software_versions"] != min(orders.values()): + raise AssertionError("Section software_versions should have the second lowest order") except (AssertionError, KeyError, TypeError) as e: failed.append(f"'assets/multiqc_config.yml' does not meet requirements: {e}") else: @@ -72,11 +73,15 @@ def multiqc_config(self): if "report_comment" not in ignore_configs: # Check that the minimum plugins exist and are coming first in the summary try: - assert "report_comment" in mqc_yml - assert ( - mqc_yml["report_comment"].strip() - == f'This report has been generated by the nf-core/{self.pipeline_name} analysis pipeline. For information about how to interpret these results, please see the documentation.' - ) + if "report_comment" not in mqc_yml: + raise AssertionError() + if mqc_yml["report_comment"].strip() != ( + f'This report has been generated by the nf-core/{self.pipeline_name} analysis pipeline. For information about how to ' + f'interpret these results, please see the documentation.' + ): + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append("'assets/multiqc_config.yml' does not contain a matching 'report_comment'.") else: @@ -84,8 +89,10 @@ def multiqc_config(self): # Check that export_plots is activated try: - assert "export_plots" in mqc_yml - assert mqc_yml["export_plots"] == True + if "export_plots" not in mqc_yml: + raise AssertionError() + if not mqc_yml["export_plots"]: + raise AssertionError() except (AssertionError, KeyError, TypeError): failed.append("'assets/multiqc_config.yml' does not contain 'export_plots: true'.") else: diff --git a/nf_core/lint/nextflow_config.py b/nf_core/lint/nextflow_config.py index bb018832bd..1f5e94c4cc 100644 --- a/nf_core/lint/nextflow_config.py +++ b/nf_core/lint/nextflow_config.py @@ -206,24 +206,24 @@ def nextflow_config(self): if "manifest.name" not in ignore_configs: # Check that the pipeline name starts with nf-core try: - assert self.nf_config.get("manifest.name", "").strip("'\"").startswith("nf-core/") + manifest_name = self.nf_config.get("manifest.name", "").strip("'\"") + if not manifest_name.startswith("nf-core/"): + raise AssertionError() except (AssertionError, IndexError): - failed.append( - "Config ``manifest.name`` did not begin with ``nf-core/``:\n {}".format( - self.nf_config.get("manifest.name", "").strip("'\"") - ) - ) + failed.append("Config ``manifest.name`` did not begin with ``nf-core/``:\n {}".format(manifest_name)) else: passed.append("Config ``manifest.name`` began with ``nf-core/``") if "manifest.homePage" not in ignore_configs: # Check that the homePage is set to the GitHub URL try: - assert self.nf_config.get("manifest.homePage", "").strip("'\"").startswith("https://github.com/nf-core/") + manifest_homepage = self.nf_config.get("manifest.homePage", "").strip("'\"") + if not manifest_homepage.startswith("https://github.com/nf-core/"): + raise AssertionError() except (AssertionError, IndexError): failed.append( "Config variable ``manifest.homePage`` did not begin with https://github.com/nf-core/:\n {}".format( - self.nf_config.get("manifest.homePage", "").strip("'\"") + manifest_homepage ) ) else: diff --git a/nf_core/lint/readme.py b/nf_core/lint/readme.py index 9099982771..99def0a204 100644 --- a/nf_core/lint/readme.py +++ b/nf_core/lint/readme.py @@ -49,7 +49,8 @@ def readme(self): if match: nf_badge_version = match.group(1).strip("'\"") try: - assert nf_badge_version == self.minNextflowVersion + if nf_badge_version != self.minNextflowVersion: + raise AssertionError() except (AssertionError, KeyError): failed.append( f"README Nextflow minimum version badge does not match config. Badge: `{nf_badge_version}`, " @@ -70,7 +71,8 @@ def readme(self): if match: nf_quickstart_version = match.group(1) try: - assert nf_quickstart_version == self.minNextflowVersion + if nf_quickstart_version != self.minNextflowVersion: + raise AssertionError() except (AssertionError, KeyError): failed.append( f"README Nextflow minimium version in Quick Start section does not match config. README: `{nf_quickstart_version}`, Config `{self.minNextflowVersion}`" diff --git a/nf_core/pipeline-template/bin/check_samplesheet.py b/nf_core/pipeline-template/bin/check_samplesheet.py index 0904481506..9a8b896239 100755 --- a/nf_core/pipeline-template/bin/check_samplesheet.py +++ b/nf_core/pipeline-template/bin/check_samplesheet.py @@ -78,13 +78,15 @@ def validate_and_transform(self, row): def _validate_sample(self, row): """Assert that the sample name exists and convert spaces to underscores.""" - assert len(row[self._sample_col]) > 0, "Sample input is required." + if len(row[self._sample_col]) <= 0: + raise AssertionError("Sample input is required.") # Sanitize samples slightly. row[self._sample_col] = row[self._sample_col].replace(" ", "_") def _validate_first(self, row): """Assert that the first FASTQ entry is non-empty and has the right format.""" - assert len(row[self._first_col]) > 0, "At least the first FASTQ file is required." + if len(row[self._first_col]) <= 0: + raise AssertionError("At least the first FASTQ file is required.") self._validate_fastq_format(row[self._first_col]) def _validate_second(self, row): @@ -96,18 +98,18 @@ def _validate_pair(self, row): """Assert that read pairs have the same file extension. Report pair status.""" if row[self._first_col] and row[self._second_col]: row[self._single_col] = False - assert ( - Path(row[self._first_col]).suffixes[-2:] == Path(row[self._second_col]).suffixes[-2:] - ), "FASTQ pairs must have the same file extensions." + if Path(row[self._first_col]).suffixes[-2:] != Path(row[self._second_col]).suffixes[-2:]: + raise AssertionError("FASTQ pairs must have the same file extensions.") else: row[self._single_col] = True def _validate_fastq_format(self, filename): """Assert that a given filename has one of the expected FASTQ extensions.""" - assert any(filename.endswith(extension) for extension in self.VALID_FORMATS), ( - f"The FASTQ file has an unrecognized extension: {filename}\n" - f"It should be one of: {', '.join(self.VALID_FORMATS)}" - ) + if not any(filename.endswith(extension) for extension in self.VALID_FORMATS): + raise AssertionError( + f"The FASTQ file has an unrecognized extension: {filename}\n" + f"It should be one of: {', '.join(self.VALID_FORMATS)}" + ) def validate_unique_samples(self): """ @@ -117,7 +119,8 @@ def validate_unique_samples(self): number of times the same sample exist, but with different FASTQ files, e.g., multiple runs per experiment. """ - assert len(self._seen) == len(self.modified), "The pair of sample name and FASTQ must be unique." + if len(self._seen) != len(self.modified): + raise AssertionError("The pair of sample name and FASTQ must be unique.") seen = Counter() for row in self.modified: sample = row[self._sample_col] diff --git a/nf_core/pipeline-template/modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py b/nf_core/pipeline-template/modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py index ab8f4f15c6..787bdb7b1b 100644 --- a/nf_core/pipeline-template/modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py +++ b/nf_core/pipeline-template/modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py @@ -59,11 +59,12 @@ def _make_versions_html(versions): for process, process_versions in versions_by_process.items(): module = process.split(":")[-1] try: - assert versions_by_module[module] == process_versions, ( - "We assume that software versions are the same between all modules. " - "If you see this error-message it means you discovered an edge-case " - "and should open an issue in nf-core/tools. " - ) + if versions_by_module[module] != process_versions: + raise AssertionError( + "We assume that software versions are the same between all modules. " + "If you see this error-message it means you discovered an edge-case " + "and should open an issue in nf-core/tools. " + ) except KeyError: versions_by_module[module] = process_versions diff --git a/nf_core/schema.py b/nf_core/schema.py index 67571635f8..d6002fac92 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -202,12 +202,11 @@ def load_input_params(self, params_path): def validate_params(self): """Check given parameters against a schema and validate""" - try: - assert self.schema is not None - jsonschema.validate(self.input_params, self.schema) - except AssertionError: + if self.schema is None: log.error("[red][✗] Pipeline schema not found") return False + try: + jsonschema.validate(self.input_params, self.schema) except jsonschema.exceptions.ValidationError as e: log.error(f"[red][✗] Input parameters are invalid: {e.message}") return False @@ -222,8 +221,9 @@ def validate_default_params(self): Additional check that all parameters have defaults in nextflow.config and that these are valid and adhere to guidelines """ + if self.schema is None: + log.error("[red][✗] Pipeline schema not found") try: - assert self.schema is not None # Make copy of schema and remove required flags schema_no_required = copy.deepcopy(self.schema) if "required" in schema_no_required: @@ -232,8 +232,6 @@ def validate_default_params(self): if "required" in group: schema_no_required["definitions"][group_key].pop("required") jsonschema.validate(self.schema_defaults, schema_no_required) - except AssertionError: - log.error("[red][✗] Pipeline schema not found") except jsonschema.exceptions.ValidationError as e: raise AssertionError(f"Default parameters are invalid: {e.message}") log.info("[green][✓] Default parameters match schema validation") @@ -342,29 +340,33 @@ def validate_schema(self, schema=None): num_params = len(param_keys) for d_key, d_schema in schema.get("definitions", {}).items(): # Check that this definition is mentioned in allOf - assert "allOf" in schema, "Schema has definitions, but no allOf key" + if "allOf" not in schema: + raise AssertionError("Schema has definitions, but no allOf key") in_allOf = False for allOf in schema["allOf"]: if allOf["$ref"] == f"#/definitions/{d_key}": in_allOf = True - assert in_allOf, f"Definition subschema `{d_key}` not included in schema `allOf`" + if not in_allOf: + raise AssertionError(f"Definition subschema `{d_key}` not included in schema `allOf`") for d_param_id in d_schema.get("properties", {}): # Check that we don't have any duplicate parameter IDs in different definitions - assert ( - d_param_id not in param_keys - ), f"Duplicate parameter found in schema `definitions`: `{d_param_id}`" + if d_param_id in param_keys: + raise AssertionError(f"Duplicate parameter found in schema `definitions`: `{d_param_id}`") param_keys.append(d_param_id) num_params += 1 # Check that everything in allOf exists for allOf in schema.get("allOf", []): - assert "definitions" in schema, "Schema has allOf, but no definitions" + if "definitions" not in schema: + raise AssertionError("Schema has allOf, but no definitions") def_key = allOf["$ref"][14:] - assert def_key in schema["definitions"], f"Subschema `{def_key}` found in `allOf` but not `definitions`" + if def_key not in schema["definitions"]: + raise AssertionError(f"Subschema `{def_key}` found in `allOf` but not `definitions`") # Check that the schema describes at least one parameter - assert num_params > 0, "No parameters found in schema" + if num_params == 0: + raise AssertionError("No parameters found in schema") return num_params @@ -379,11 +381,11 @@ def validate_schema_title_description(self, schema=None): log.debug("Pipeline schema not set - skipping validation of top-level attributes") return None - assert "$schema" in self.schema, "Schema missing top-level `$schema` attribute" + if "$schema" not in self.schema: + raise AssertionError("Schema missing top-level `$schema` attribute") schema_attr = "http://json-schema.org/draft-07/schema" - assert ( - self.schema["$schema"] == schema_attr - ), f"Schema `$schema` should be `{schema_attr}`\n Found `{self.schema['$schema']}`" + if self.schema["$schema"] != schema_attr: + raise AssertionError(f"Schema `$schema` should be `{schema_attr}`\n Found `{self.schema['$schema']}`") if self.pipeline_manifest == {}: self.get_wf_params() @@ -391,27 +393,31 @@ def validate_schema_title_description(self, schema=None): if "name" not in self.pipeline_manifest: log.debug("Pipeline manifest `name` not known - skipping validation of schema id and title") else: - assert "$id" in self.schema, "Schema missing top-level `$id` attribute" - assert "title" in self.schema, "Schema missing top-level `title` attribute" + if "$id" not in self.schema: + raise AssertionError("Schema missing top-level `$id` attribute") + if "title" not in self.schema: + raise AssertionError("Schema missing top-level `title` attribute") # Validate that id, title and description match the pipeline manifest id_attr = "https://raw.githubusercontent.com/{}/master/nextflow_schema.json".format( self.pipeline_manifest["name"].strip("\"'") ) - assert self.schema["$id"] == id_attr, f"Schema `$id` should be `{id_attr}`\n Found `{self.schema['$id']}`" + if self.schema["$id"] != id_attr: + raise AssertionError(f"Schema `$id` should be `{id_attr}`\n Found `{self.schema['$id']}`") title_attr = "{} pipeline parameters".format(self.pipeline_manifest["name"].strip("\"'")) - assert ( - self.schema["title"] == title_attr - ), f"Schema `title` should be `{title_attr}`\n Found: `{self.schema['title']}`" + if self.schema["title"] != title_attr: + raise AssertionError(f"Schema `title` should be `{title_attr}`\n Found: `{self.schema['title']}`") if "description" not in self.pipeline_manifest: log.debug("Pipeline manifest 'description' not known - skipping validation of schema description") else: - assert "description" in self.schema, "Schema missing top-level 'description' attribute" + if "description" not in self.schema: + raise AssertionError("Schema missing top-level 'description' attribute") desc_attr = self.pipeline_manifest["description"].strip("\"'") - assert ( - self.schema["description"] == desc_attr - ), f"Schema 'description' should be '{desc_attr}'\n Found: '{self.schema['description']}'" + if self.schema["description"] != desc_attr: + raise AssertionError( + f"Schema 'description' should be '{desc_attr}'\n Found: '{self.schema['description']}'" + ) def check_for_input_mimetype(self): """ @@ -708,7 +714,7 @@ def prompt_remove_schema_notfound_config(self, p_key): Returns True if it should be removed, False if not. """ - if p_key not in self.pipeline_params.keys(): + if p_key not in self.pipeline_params: if self.no_prompts or self.schema_from_scratch: return True if Confirm.ask( @@ -766,7 +772,7 @@ def build_schema_param(self, p_val): p_val = None # Booleans - if p_val == "True" or p_val == "False": + if p_val in ["True", "False"]: p_val = p_val == "True" # Convert to bool p_type = "boolean" @@ -787,10 +793,15 @@ def launch_web_builder(self): } web_response = nf_core.utils.poll_nfcore_web_api(self.web_schema_build_url, content) try: - assert "api_url" in web_response - assert "web_url" in web_response + if "api_url" not in web_response: + raise AssertionError('"api_url" not in web_response') + if "web_url" not in web_response: + raise AssertionError('"web_url" not in web_response') # DO NOT FIX THIS TYPO. Needs to stay in sync with the website. Maintaining for backwards compatability. - assert web_response["status"] == "recieved" + if web_response["status"] != "recieved": + raise AssertionError( + f'web_response["status"] should be "recieved", but it is "{web_response["status"]}"' + ) except (AssertionError) as e: log.debug(f"Response content:\n{json.dumps(web_response, indent=4)}") raise AssertionError( @@ -813,9 +824,9 @@ def get_web_builder_response(self): web_response = nf_core.utils.poll_nfcore_web_api(self.web_schema_build_api_url) if web_response["status"] == "error": raise AssertionError(f"Got error from schema builder: '{web_response.get('message')}'") - elif web_response["status"] == "waiting_for_user": + if web_response["status"] == "waiting_for_user": return False - elif web_response["status"] == "web_builder_edited": + if web_response["status"] == "web_builder_edited": log.info("Found saved status from nf-core schema builder") try: self.schema = web_response["schema"] diff --git a/nf_core/utils.py b/nf_core/utils.py index 822d5a6811..48817adfc3 100644 --- a/nf_core/utils.py +++ b/nf_core/utils.py @@ -389,7 +389,8 @@ def poll_nfcore_web_api(api_url, post_data=None): else: try: web_response = json.loads(response.content) - assert "status" in web_response + if "status" not in web_response: + raise AssertionError() except (json.decoder.JSONDecodeError, AssertionError, TypeError) as e: log.debug(f"Response content:\n{response.content}") raise AssertionError( @@ -877,9 +878,8 @@ def get_repo_releases_branches(pipeline, wfs): # Check that this repo existed try: - assert rel_r.json().get("message") != "Not Found" - except AssertionError: - raise AssertionError(f"Not able to find pipeline '{pipeline}'") + if rel_r.json().get("message") == "Not Found": + raise AssertionError(f"Not able to find pipeline '{pipeline}'") except AttributeError: # Success! We have a list, which doesn't work with .get() which is looking for a dict key wf_releases = list(sorted(rel_r.json(), key=lambda k: k.get("published_at_timestamp", 0), reverse=True)) From 2b90479baff6d5e0daa27e534fbeaa6d46612bc1 Mon Sep 17 00:00:00 2001 From: Fabian Egli Date: Thu, 21 Jul 2022 09:59:18 +0200 Subject: [PATCH 2/4] use f-string instead of format Co-authored-by: Erik Danielsson <53212377+ErikDanielsson@users.noreply.github.com> --- nf_core/lint/nextflow_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/lint/nextflow_config.py b/nf_core/lint/nextflow_config.py index 1f5e94c4cc..635e33cfb5 100644 --- a/nf_core/lint/nextflow_config.py +++ b/nf_core/lint/nextflow_config.py @@ -210,7 +210,7 @@ def nextflow_config(self): if not manifest_name.startswith("nf-core/"): raise AssertionError() except (AssertionError, IndexError): - failed.append("Config ``manifest.name`` did not begin with ``nf-core/``:\n {}".format(manifest_name)) + failed.append(f"Config ``manifest.name`` did not begin with ``nf-core/``:\n {manifest_name}") else: passed.append("Config ``manifest.name`` began with ``nf-core/``") From 8309dadffbf6d6ec554e7276e5bce25572d753a0 Mon Sep 17 00:00:00 2001 From: fabianegli Date: Thu, 21 Jul 2022 10:04:30 +0200 Subject: [PATCH 3/4] make program control flow optimization resisant --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a74672fe37..e59cf9604f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - Remove support for Python 3.6 ([#1680](https://github.com/nf-core/tools/pull/1680)) - Add support for Python 3.9 and 3.10 ([#1680](https://github.com/nf-core/tools/pull/1680)) +- Programm control flow no longer affected when run with optimizations ([#1685](https://github.com/nf-core/tools/pull/1685)) - Update `readme` to drop `--key` option from `nf-core modules list` and add the new pattern syntax - Add `--fail-warned` flag to `nf-core lint` to make warnings fail ([#1593](https://github.com/nf-core/tools/pull/1593)) - Add `--fail-warned` flag to pipeline linting workflow ([#1593](https://github.com/nf-core/tools/pull/1593)) From 47878157c3ff0f8fa977bc3487c8f9fa8adf127e Mon Sep 17 00:00:00 2001 From: Fabian Egli Date: Thu, 21 Jul 2022 12:28:22 +0200 Subject: [PATCH 4/4] better wording for assert removal --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e59cf9604f..e9777d3c3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ - Remove support for Python 3.6 ([#1680](https://github.com/nf-core/tools/pull/1680)) - Add support for Python 3.9 and 3.10 ([#1680](https://github.com/nf-core/tools/pull/1680)) -- Programm control flow no longer affected when run with optimizations ([#1685](https://github.com/nf-core/tools/pull/1685)) +- Invoking Python with optimizations no longer affects the program control flow ([#1685](https://github.com/nf-core/tools/pull/1685)) - Update `readme` to drop `--key` option from `nf-core modules list` and add the new pattern syntax - Add `--fail-warned` flag to `nf-core lint` to make warnings fail ([#1593](https://github.com/nf-core/tools/pull/1593)) - Add `--fail-warned` flag to pipeline linting workflow ([#1593](https://github.com/nf-core/tools/pull/1593))