Merge pull request #1685 from fabianegli/remove-assert-from-productio…

…n-code remove assert and code cleanup
nf-core · Jul 21, 2022 · c5a4295 · c5a4295
2 parents dc667c9 + 4787815
commit c5a4295
Show file tree

Hide file tree

Showing 13 changed files with 160 additions and 118 deletions.
diff --git 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))
+- 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))

diff --git 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="<pipeline schema>")
+@click.argument(
+    "schema_path",
+    type=click.Path(exists=True),
+    default="nextflow_schema.json",
+    required=False,
+    metavar="<pipeline schema>",
+)
 @click.option("-o", "--output", type=str, metavar="<filename>", 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,20 +73,26 @@ 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 <a href="https://github.com/nf-core/{self.pipeline_name}" target="_blank">nf-core/{self.pipeline_name}</a> analysis pipeline. For information about how to interpret these results, please see the <a href="https://nf-co.re/{self.pipeline_name}" target="_blank">documentation</a>.'
-            )
+            if "report_comment" not in mqc_yml:
+                raise AssertionError()
+            if mqc_yml["report_comment"].strip() != (
+                f'This report has been generated by the <a href="https://github.com/nf-core/{self.pipeline_name}" '
+                f'target="_blank">nf-core/{self.pipeline_name}</a> analysis pipeline. For information about how to '
+                f'interpret these results, please see the <a href="https://nf-co.re/{self.pipeline_name}" '
+                'target="_blank">documentation</a>.'
+            ):
+                raise AssertionError()
         except (AssertionError, KeyError, TypeError):
             failed.append("'assets/multiqc_config.yml' does not contain a matching 'report_comment'.")
         else:
             passed.append("'assets/multiqc_config.yml' contains a matching 'report_comment'.")
 
     # 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
@@ -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(f"Config ``manifest.name`` did not begin with ``nf-core/``:\n    {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
@@ -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
@@ -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/...ate/modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py b/...ate/modules/nf-core/modules/custom/dumpsoftwareversions/templates/dumpsoftwareversions.py