Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove assert and code cleanup #1685

Merged
merged 4 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand All @@ -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")
Expand Down
14 changes: 10 additions & 4 deletions nf_core/launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions nf_core/lint/actions_awsfulltest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
7 changes: 4 additions & 3 deletions nf_core/lint/actions_awstest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 16 additions & 9 deletions nf_core/lint/actions_ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
43 changes: 25 additions & 18 deletions nf_core/lint/multiqc_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions nf_core/lint/nextflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
fabianegli marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand Down
6 changes: 4 additions & 2 deletions nf_core/lint/readme.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}`, "
Expand All @@ -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}`"
Expand Down
23 changes: 13 additions & 10 deletions nf_core/pipeline-template/bin/check_samplesheet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
"""
Expand All @@ -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]
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading