From 7f686b49113be151ab034c29245a9ce460bed25f Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Sat, 30 Nov 2024 13:07:55 +0100 Subject: [PATCH] argpase: colorize the --help output with rich-argparse --- .github/workflows/ci-tests.yml | 2 +- cwltool/argparser.py | 352 ++++++++++++++++++--------------- cwltool/main.py | 7 +- pyproject.toml | 1 + requirements.txt | 1 + setup.py | 1 + tests/test_environment.py | 12 +- tests/test_misc_cli.py | 10 +- tests/test_singularity.py | 3 +- 9 files changed, 215 insertions(+), 174 deletions(-) diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 6608e9a22..1f01160c8 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -157,7 +157,7 @@ jobs: chmod a-w . - name: run tests - run: APPTAINER_TMPDIR=${RUNNER_TEMP} make test + run: APPTAINER_TMPDIR=${RUNNER_TEMP} make test PYTEST_EXTRA=-vvv conformance_tests: diff --git a/cwltool/argparser.py b/cwltool/argparser.py index 7b3125d94..4711b3c72 100644 --- a/cwltool/argparser.py +++ b/cwltool/argparser.py @@ -6,6 +6,9 @@ from collections.abc import MutableMapping, MutableSequence, Sequence from typing import Any, Callable, Optional, Union, cast +import rich.markup +from rich_argparse import HelpPreviewAction, RichHelpFormatter + from .loghandler import _logger from .process import Process, shortname from .resolver import ga4gh_tool_registries @@ -14,9 +17,11 @@ def arg_parser() -> argparse.ArgumentParser: + RichHelpFormatter.group_name_formatter = str parser = argparse.ArgumentParser( + formatter_class=RichHelpFormatter, description="Reference executor for Common Workflow Language standards. " - "Not for production use." + "Not for production use.", ) parser.add_argument("--basedir", type=str) parser.add_argument( @@ -26,23 +31,15 @@ def arg_parser() -> argparse.ArgumentParser: help="Output directory. The default is the current directory.", ) - parser.add_argument( - "--log-dir", - type=str, - default="", - help="Log your tools stdout/stderr to this location outside of container " - "This will only log stdout/stderr if you specify stdout/stderr in their " - "respective fields or capture it as an output", - ) - parser.add_argument( "--parallel", action="store_true", default=False, help="Run jobs in parallel. ", ) - envgroup = parser.add_mutually_exclusive_group() - envgroup.add_argument( + envgroup = parser.add_argument_group(title="Control environement variables") + env_exclusive = envgroup.add_mutually_exclusive_group() + env_exclusive.add_argument( "--preserve-environment", type=str, action="append", @@ -53,7 +50,7 @@ def arg_parser() -> argparse.ArgumentParser: default=[], dest="preserve_environment", ) - envgroup.add_argument( + env_exclusive.add_argument( "--preserve-entire-environment", action="store_true", help="Preserve all environment variables when running CommandLineTools " @@ -62,54 +59,10 @@ def arg_parser() -> argparse.ArgumentParser: dest="preserve_entire_environment", ) - containergroup = parser.add_mutually_exclusive_group() - containergroup.add_argument( - "--rm-container", - action="store_true", - default=True, - help="Delete Docker container used by jobs after they exit (default)", - dest="rm_container", - ) - - containergroup.add_argument( - "--leave-container", - action="store_false", - default=True, - help="Do not delete Docker container used by jobs after they exit", - dest="rm_container", - ) - - cidgroup = parser.add_argument_group( - "Options for recording the Docker container identifier into a file." - ) - cidgroup.add_argument( - # Disabled as containerid is now saved by default - "--record-container-id", - action="store_true", - default=False, - help=argparse.SUPPRESS, - dest="record_container_id", - ) - - cidgroup.add_argument( - "--cidfile-dir", - type=str, - help="Store the Docker container ID into a file in the specified directory.", - default=None, - dest="cidfile_dir", - ) - - cidgroup.add_argument( - "--cidfile-prefix", - type=str, - help="Specify a prefix to the container ID filename. " - "Final file name will be followed by a timestamp. " - "The default is no prefix.", - default=None, - dest="cidfile_prefix", + files_group = parser.add_argument_group( + title="Manage intermediate, temporary, or final output files" ) - - parser.add_argument( + files_group.add_argument( "--tmpdir-prefix", type=str, help="Path prefix for temporary directories. If --tmpdir-prefix is not " @@ -119,7 +72,7 @@ def arg_parser() -> argparse.ArgumentParser: default=DEFAULT_TMP_PREFIX, ) - intgroup = parser.add_mutually_exclusive_group() + intgroup = files_group.add_mutually_exclusive_group() intgroup.add_argument( "--tmp-outdir-prefix", type=str, @@ -137,7 +90,7 @@ def arg_parser() -> argparse.ArgumentParser: "troubleshooting of CWL documents.", ) - tmpgroup = parser.add_mutually_exclusive_group() + tmpgroup = files_group.add_mutually_exclusive_group() tmpgroup.add_argument( "--rm-tmpdir", action="store_true", @@ -154,7 +107,7 @@ def arg_parser() -> argparse.ArgumentParser: dest="rm_tmpdir", ) - outgroup = parser.add_mutually_exclusive_group() + outgroup = files_group.add_mutually_exclusive_group() outgroup.add_argument( "--move-outputs", action="store_const", @@ -184,30 +137,6 @@ def arg_parser() -> argparse.ArgumentParser: dest="move_outputs", ) - pullgroup = parser.add_mutually_exclusive_group() - pullgroup.add_argument( - "--enable-pull", - default=True, - action="store_true", - help="Try to pull Docker images", - dest="pull_image", - ) - - pullgroup.add_argument( - "--disable-pull", - default=True, - action="store_false", - help="Do not try to pull Docker images", - dest="pull_image", - ) - - parser.add_argument( - "--rdf-serializer", - help="Output RDF serialization format used by --print-rdf (one of " - "turtle (default), n3, nt, xml)", - default="turtle", - ) - parser.add_argument( "--eval-timeout", help="Time to wait for a Javascript expression to evaluate before giving " @@ -216,9 +145,7 @@ def arg_parser() -> argparse.ArgumentParser: default=60, ) - provgroup = parser.add_argument_group( - "Options for recording provenance information of the execution" - ) + provgroup = parser.add_argument_group("Recording provenance information of the execution") provgroup.add_argument( "--provenance", help="Save provenance to specified folder as a " @@ -276,7 +203,8 @@ def arg_parser() -> argparse.ArgumentParser: type=str, ) - printgroup = parser.add_mutually_exclusive_group() + non_exec_group = parser.add_argument_group(title="Non-execution options") + printgroup = non_exec_group.add_mutually_exclusive_group() printgroup.add_argument( "--print-rdf", action="store_true", @@ -324,6 +252,15 @@ def arg_parser() -> argparse.ArgumentParser: printgroup.add_argument( "--make-template", action="store_true", help="Generate a template input object" ) + non_exec_group.add_argument( + "--rdf-serializer", + help="Output RDF serialization format used by --print-rdf (one of " + "turtle (default), n3, nt, xml)", + default="turtle", + ) + non_exec_group.add_argument( + "--tool-help", action="store_true", help="Print command line help for tool" + ) strictgroup = parser.add_mutually_exclusive_group() strictgroup.add_argument( @@ -365,11 +302,27 @@ def arg_parser() -> argparse.ArgumentParser: dest="doc_cache", ) - volumegroup = parser.add_mutually_exclusive_group() - volumegroup.add_argument("--verbose", action="store_true", help="Default logging") - volumegroup.add_argument("--no-warnings", action="store_true", help="Only print errors.") - volumegroup.add_argument("--quiet", action="store_true", help="Only print warnings and errors.") - volumegroup.add_argument("--debug", action="store_true", help="Print even more logging") + volumegroup = parser.add_argument_group(title="Configure logging") + volume_exclusive = volumegroup.add_mutually_exclusive_group() + volume_exclusive.add_argument("--verbose", action="store_true", help="Default logging") + volume_exclusive.add_argument("--no-warnings", action="store_true", help="Only print errors.") + volume_exclusive.add_argument( + "--quiet", action="store_true", help="Only print warnings and errors." + ) + volume_exclusive.add_argument("--debug", action="store_true", help="Print even more logging") + volumegroup.add_argument( + "--log-dir", + type=str, + default="", + help="Log your tools stdout/stderr to this location outside of container " + "This will only log stdout/stderr if you specify stdout/stderr in their " + "respective fields or capture it as an output", + ) + volumegroup.add_argument( + "--timestamps", + action="store_true", + help="Add timestamps to the errors, warnings, and notifications.", + ) parser.add_argument( "--write-summary", @@ -380,30 +333,6 @@ def arg_parser() -> argparse.ArgumentParser: dest="write_summary", ) - parser.add_argument( - "--strict-memory-limit", - action="store_true", - help="When running with " - "software containers and the Docker engine, pass either the " - "calculated memory allocation from ResourceRequirements or the " - "default of 1 gigabyte to Docker's --memory option.", - ) - - parser.add_argument( - "--strict-cpu-limit", - action="store_true", - help="When running with " - "software containers and the Docker engine, pass either the " - "calculated cpu allocation from ResourceRequirements or the " - "default of 1 core to Docker's --cpu option. " - "Requires docker version >= v1.13.", - ) - - parser.add_argument( - "--timestamps", - action="store_true", - help="Add timestamps to the errors, warnings, and notifications.", - ) parser.add_argument( "--js-console", action="store_true", help="Enable javascript console output" ) @@ -418,7 +347,105 @@ def arg_parser() -> argparse.ArgumentParser: help="File of options to pass to jshint. " 'This includes the added option "includewarnings". ', ) - dockergroup = parser.add_mutually_exclusive_group() + container_group = parser.add_argument_group( + title="Software container engine selection and configuration" + ) + pullgroup = container_group.add_mutually_exclusive_group() + pullgroup.add_argument( + "--enable-pull", + default=True, + action="store_true", + help="Try to pull Docker images", + dest="pull_image", + ) + + pullgroup.add_argument( + "--disable-pull", + default=True, + action="store_false", + help="Do not try to pull Docker images", + dest="pull_image", + ) + container_group.add_argument( + "--force-docker-pull", + action="store_true", + default=False, + help="Pull latest software container image even if it is locally present", + dest="force_docker_pull", + ) + container_group.add_argument( + "--no-read-only", + action="store_true", + default=False, + help="Do not set root directory in the container as read-only", + dest="no_read_only", + ) + + container_group.add_argument( + "--default-container", + help="Specify a default software container to use for any " + "CommandLineTool without a DockerRequirement.", + ) + container_group.add_argument( + "--no-match-user", + action="store_true", + help="Disable passing the current uid to `docker run --user`", + ) + container_group.add_argument( + "--custom-net", + type=str, + help="Passed to `docker run` as the `--net` parameter when " + "NetworkAccess is true, which is its default setting.", + ) + + container_cleanup = container_group.add_mutually_exclusive_group() + container_cleanup.add_argument( + "--rm-container", + action="store_true", + default=True, + help="Delete Docker container used by jobs after they exit (default)", + dest="rm_container", + ) + + container_cleanup.add_argument( + "--leave-container", + action="store_false", + default=True, + help="Do not delete Docker container used by jobs after they exit", + dest="rm_container", + ) + + cidgroup = container_group.add_argument_group( + "Recording the Docker container identifier into a file" + ) + cidgroup.add_argument( + # Disabled as containerid is now saved by default + "--record-container-id", + action="store_true", + default=False, + help=argparse.SUPPRESS, + dest="record_container_id", + ) + + cidgroup.add_argument( + "--cidfile-dir", + type=str, + help="Store the Docker container ID into a file in the specified directory.", + default=None, + dest="cidfile_dir", + ) + + cidgroup.add_argument( + "--cidfile-prefix", + type=str, + help="Specify a prefix to the container ID filename. " + "Final file name will be followed by a timestamp. " + "The default is no prefix.", + default=None, + dest="cidfile_prefix", + ) + + dockergroup = container_group.add_mutually_exclusive_group() dockergroup.add_argument( "--user-space-docker-cmd", metavar="CMD", @@ -458,6 +485,24 @@ def arg_parser() -> argparse.ArgumentParser: "is specified under `hints`.", dest="use_container", ) + container_group.add_argument( + "--strict-memory-limit", + action="store_true", + help="When running with " + "software containers and the Docker engine, pass either the " + "calculated memory allocation from ResourceRequirements or the " + "default of 1 gigabyte to Docker's --memory option.", + ) + + container_group.add_argument( + "--strict-cpu-limit", + action="store_true", + help="When running with " + "software containers and the Docker engine, pass either the " + "calculated cpu allocation from ResourceRequirements or the " + "default of 1 core to Docker's --cpu option. " + "Requires docker version >= v1.13.", + ) dependency_resolvers_configuration_help = argparse.SUPPRESS dependencies_directory_help = argparse.SUPPRESS @@ -467,7 +512,7 @@ def arg_parser() -> argparse.ArgumentParser: if SOFTWARE_REQUIREMENTS_ENABLED: dependency_resolvers_configuration_help = ( "Dependency resolver " - "configuration file describing how to adapt 'SoftwareRequirement' " + "configuration file describing how to adapt `SoftwareRequirement` " "packages to current system." ) dependencies_directory_help = ( @@ -476,7 +521,7 @@ def arg_parser() -> argparse.ArgumentParser: use_biocontainers_help = ( "Use biocontainers for tools without an " "explicitly annotated Docker container." ) - conda_dependencies = "Short cut to use Conda to resolve 'SoftwareRequirement' packages." + conda_dependencies = "Short cut to use Conda to resolve `SoftwareRequirement` packages." parser.add_argument( "--beta-dependency-resolvers-configuration", @@ -499,8 +544,6 @@ def arg_parser() -> argparse.ArgumentParser: action="store_true", ) - parser.add_argument("--tool-help", action="store_true", help="Print command line help for tool") - parser.add_argument( "--relative-deps", choices=["primary", "cwd"], @@ -519,7 +562,7 @@ def arg_parser() -> argparse.ArgumentParser: parser.add_argument( "--enable-ext", action="store_true", - help="Enable loading and running 'cwltool:' extensions to the CWL standards.", + help="Enable loading and running `cwltool:` extensions to the CWL standards.", default=False, ) @@ -537,22 +580,6 @@ def arg_parser() -> argparse.ArgumentParser: help="Disable colored logging (default false)", ) - parser.add_argument( - "--default-container", - help="Specify a default software container to use for any " - "CommandLineTool without a DockerRequirement.", - ) - parser.add_argument( - "--no-match-user", - action="store_true", - help="Disable passing the current uid to `docker run --user`", - ) - parser.add_argument( - "--custom-net", - type=str, - help="Passed to `docker run` as the '--net' parameter when " - "NetworkAccess is true, which is its default setting.", - ) parser.add_argument( "--disable-validate", dest="do_validate", @@ -595,9 +622,9 @@ def arg_parser() -> argparse.ArgumentParser: parser.add_argument( "--on-error", - help="Desired workflow behavior when a step fails. One of 'stop' (do " - "not submit any more steps) or 'continue' (may submit other steps that " - "are not downstream from the error). Default is 'stop'.", + help="Desired workflow behavior when a step fails. One of `stop` (do " + "not submit any more steps) or `continue` (may submit other steps that " + "are not downstream from the error). Default is `stop`.", default="stop", choices=("stop", "continue"), ) @@ -625,21 +652,6 @@ def arg_parser() -> argparse.ArgumentParser: dest="relax_path_checks", ) - parser.add_argument( - "--force-docker-pull", - action="store_true", - default=False, - help="Pull latest software container image even if it is locally present", - dest="force_docker_pull", - ) - parser.add_argument( - "--no-read-only", - action="store_true", - default=False, - help="Do not set root directory in the container as read-only", - dest="no_read_only", - ) - parser.add_argument( "--overrides", type=str, @@ -647,7 +659,8 @@ def arg_parser() -> argparse.ArgumentParser: help="Read process requirement overrides from file.", ) - subgroup = parser.add_mutually_exclusive_group() + target_group = parser.add_argument_group(title="Target selection (optional)") + subgroup = target_group.add_mutually_exclusive_group() subgroup.add_argument( "--target", "-t", @@ -668,8 +681,8 @@ def arg_parser() -> argparse.ArgumentParser: default=None, help="Only executes the underlying Process (CommandLineTool, " "ExpressionTool, or sub-Workflow) for the given step in a workflow. " - "This will not include any step-level processing: 'scatter', 'when'; " - "and there will be no processing of step-level 'default', or 'valueFrom' " + "This will not include any step-level processing: `scatter`, `when`; " + "and there will be no processing of step-level `default`, or `valueFrom` " "input modifiers. However, requirements/hints from the step or parent " "workflow(s) will be inherited as usual." "The input object must match that Process's inputs.", @@ -703,7 +716,11 @@ def arg_parser() -> argparse.ArgumentParser: "formatted description of the required input values for the given " "`cwl_document`.", ) - + parser.add_argument( + "--generate-help-preview", + action=HelpPreviewAction, + path="help-preview.svg", # (optional) or "help-preview.html" or "help-preview.txt" + ) return parser @@ -855,6 +872,7 @@ def add_argument( urljoin: Callable[[str, str], str] = urllib.parse.urljoin, base_uri: str = "", ) -> None: + description = rich.markup.escape(description) if len(name) == 1: flag = "-" else: @@ -980,4 +998,10 @@ def generate_parser( base_uri, ) + toolparser.add_argument( + "--generate-help-preview", + action=HelpPreviewAction, + path="help-preview.svg", # (optional) or "help-preview.html" or "help-preview.txt" + ) + return toolparser diff --git a/cwltool/main.py b/cwltool/main.py index 17ccb11ce..b7ba40d40 100755 --- a/cwltool/main.py +++ b/cwltool/main.py @@ -22,6 +22,7 @@ import coloredlogs import requests import ruamel.yaml +from rich_argparse import RichHelpFormatter from ruamel.yaml.comments import CommentedMap, CommentedSeq from ruamel.yaml.main import YAML from schema_salad.exceptions import ValidationException @@ -413,7 +414,10 @@ def init_job_order( namemap: dict[str, str] = {} records: list[str] = [] toolparser = generate_parser( - argparse.ArgumentParser(prog=args.workflow), + argparse.ArgumentParser( + prog=args.workflow, + formatter_class=RichHelpFormatter, + ), process, namemap, records, @@ -976,6 +980,7 @@ def main( user_agent += f" {progname}" # append the real program name as well append_word_to_default_user_agent(user_agent) + err_handler: logging.Handler = defaultStreamHandler try: if args is None: if argsl is None: diff --git a/pyproject.toml b/pyproject.toml index 7ddf547f2..248c0e69d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,7 @@ requires = [ "cwl-utils>=0.32", "toml", "argcomplete>=1.12.0", + "rich-argparse" ] build-backend = "setuptools.build_meta" diff --git a/requirements.txt b/requirements.txt index 3ac631838..3cbcf0027 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,3 +12,4 @@ argcomplete>=1.12.0 pyparsing!=3.0.2 # breaks --print-dot (pydot) https://github.com/pyparsing/pyparsing/issues/319 cwl-utils>=0.32 spython>=0.3.0 +rich-argparse diff --git a/setup.py b/setup.py index 9bbee10f1..d3fef7b26 100644 --- a/setup.py +++ b/setup.py @@ -135,6 +135,7 @@ "pyparsing != 3.0.2", # breaks --print-dot (pydot) https://github.com/pyparsing/pyparsing/issues/319 "cwl-utils >= 0.32", "spython >= 0.3.0", + "rich-argparse", ], extras_require={ "deps": [ diff --git a/tests/test_environment.py b/tests/test_environment.py index 488477aa7..4e9c602f1 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -4,7 +4,7 @@ from abc import ABC, abstractmethod from collections.abc import Mapping from pathlib import Path -from typing import Any, Callable, Union +from typing import Callable, Union import pytest @@ -198,7 +198,7 @@ def BIND(v: str, env: Env) -> bool: @CRT_PARAMS -def test_basic(crt_params: CheckHolder, tmp_path: Path, monkeypatch: Any) -> None: +def test_basic(crt_params: CheckHolder, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Test that basic env vars (only) show up.""" tmp_prefix = str(tmp_path / "canary") extra_env = { @@ -218,7 +218,9 @@ def test_basic(crt_params: CheckHolder, tmp_path: Path, monkeypatch: Any) -> Non @CRT_PARAMS -def test_preserve_single(crt_params: CheckHolder, tmp_path: Path, monkeypatch: Any) -> None: +def test_preserve_single( + crt_params: CheckHolder, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: """Test that preserving a single env var works.""" tmp_prefix = str(tmp_path / "canary") extra_env = { @@ -242,7 +244,9 @@ def test_preserve_single(crt_params: CheckHolder, tmp_path: Path, monkeypatch: A @CRT_PARAMS -def test_preserve_all(crt_params: CheckHolder, tmp_path: Path, monkeypatch: Any) -> None: +def test_preserve_all( + crt_params: CheckHolder, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: """Test that preserving all works.""" tmp_prefix = str(tmp_path / "canary") extra_env = { diff --git a/tests/test_misc_cli.py b/tests/test_misc_cli.py index 307153e16..be314cad4 100644 --- a/tests/test_misc_cli.py +++ b/tests/test_misc_cli.py @@ -1,5 +1,7 @@ """Tests for various command line options.""" +import pytest + from cwltool.utils import versionstring from .util import get_data, get_main_output, needs_docker @@ -26,9 +28,13 @@ def test_empty_cmdling() -> None: assert "CWL document required, no input file was provided" in stderr -def test_tool_help() -> None: +def test_tool_help(monkeypatch: pytest.MonkeyPatch) -> None: """Test --tool-help.""" - return_code, stdout, stderr = get_main_output(["--tool-help", get_data("tests/echo.cwl")]) + return_code, stdout, stderr = get_main_output( + ["--tool-help", get_data("tests/echo.cwl")], + extra_env={"NO_COLOR": "1"}, + monkeypatch=monkeypatch, + ) assert return_code == 0 assert "job_order Job input json file" in stdout diff --git a/tests/test_singularity.py b/tests/test_singularity.py index 0512f2e28..1139dfbc7 100644 --- a/tests/test_singularity.py +++ b/tests/test_singularity.py @@ -2,7 +2,6 @@ import shutil from pathlib import Path -from typing import Any import pytest @@ -19,7 +18,7 @@ @needs_singularity_2_6 -def test_singularity_pullfolder(tmp_path: Path, monkeypatch: Any) -> None: +def test_singularity_pullfolder(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Test singularity respects SINGULARITY_PULLFOLDER.""" workdir = tmp_path / "working_dir_new" workdir.mkdir()