From 292b91f0de72ff412583bee274689e16c41770f9 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Wed, 12 Aug 2026 15:28:05 +0100 Subject: [PATCH 1/2] #39 Make generation the default CLI action, copy via a switch Flatten the generate/copy-base-classes subcommands introduced in #39 back to a single command: generating from an SBML file is the default, and --copy-base-classes switches to copying the C++ base classes instead. The SBML file becomes an optional positional; an SBML file is required unless --copy-base-classes is given, and --copy-base-classes rejects a stray SBML file. Update the CLI tests, the README usage section, and the pyproject.toml comment to match. Co-Authored-By: Claude Opus 4.8 --- README.md | 64 +++++++++++++------------------ chaste_sbml/__main__.py | 48 ++++++++++++----------- chaste_sbml/tests/test_cli.py | 31 +++++++++++---- chaste_sbml/tests/test_console.py | 47 +++++++++-------------- pyproject.toml | 2 +- 5 files changed, 95 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index 345f72ee..28b13716 100644 --- a/README.md +++ b/README.md @@ -61,69 +61,59 @@ python3 -m pytest ## Usage -`chaste-sbml` has two subcommands: +Generating Chaste C++ code from an SBML file is the default action. Passing +`--copy-base-classes` switches to copying the C++ base classes instead. ``` -usage: chaste-sbml [-h] [--version] command ... - -positional arguments: - command - generate Generate Chaste C++ code from an SBML file - copy-base-classes Copy the C++ base classes the generated code depends on - -options: - -h, --help show this help message and exit - --version show program's version number and exit -``` - -### `generate` - -Generate Chaste C++ code from an SBML file: - -```sh -chaste-sbml generate my_model.xml --model-type srn --output-dir src/ -``` - -``` -usage: chaste-sbml generate [-h] [--output-dir OUTPUT_DIR] - [--model-type [{generic,srn,cell-cycle}]] - [--tests | --no-tests] - [--test-output-dir TEST_OUTPUT_DIR] - sbml_file +usage: chaste-sbml [-h] [--version] [--copy-base-classes] + [--output-dir OUTPUT_DIR] + [--model-type [{generic,srn,cell-cycle}]] + [--tests | --no-tests] [--timescale {ms,s,m,h}] + [--test-output-dir TEST_OUTPUT_DIR] + [sbml_file] positional arguments: sbml_file The SBML file to convert options: + -h, --help show this help message and exit + --version show program's version number and exit + --copy-base-classes Copy the C++ base classes the generated code depends + on, instead of generating code --output-dir OUTPUT_DIR The directory to place output files in --model-type [{generic,srn,cell-cycle}] The type of model to generate --tests, --no-tests Generate placeholder test files (default: on) + --timescale {ms,s,m,h} + The model's native time unit, used to convert + derivatives to Chaste's hours (auto-detected if omitted) --test-output-dir TEST_OUTPUT_DIR The directory to place generated test files in (defaults to --output-dir) ``` -By default `generate` also emits a placeholder test (`TestSbml.hpp`), a +### Generate code + +Generate Chaste C++ code from an SBML file: + +```sh +chaste-sbml my_model.xml --model-type srn --output-dir src/ +``` + +By default this also emits a placeholder test (`TestSbml.hpp`), a CxxTest skeleton with a suite for the ODE system, and the SRN/cell-cycle model where applicable. Pass `--no-tests` to skip it, or `--test-output-dir` to place the placeholder test somewhere other than `--output-dir`. -### `copy-base-classes` +### Copy the base classes The generated code `#include`s and subclasses a set of C++ base classes (for example `AbstractSbmlOdeSystem`). These are shipped with the package; copy them into your project so they match the installed version of `chaste-sbml`: ```sh -chaste-sbml copy-base-classes --output-dir src/ -``` - +chaste-sbml --copy-base-classes --output-dir src/ ``` -usage: chaste-sbml copy-base-classes [-h] [--output-dir OUTPUT_DIR] -options: - --output-dir OUTPUT_DIR - The directory to place the base classes in -``` +In this mode no SBML file is taken and the only other option that applies is `--output-dir`. diff --git a/chaste_sbml/__main__.py b/chaste_sbml/__main__.py index 78f3146d..845eaf94 100644 --- a/chaste_sbml/__main__.py +++ b/chaste_sbml/__main__.py @@ -9,24 +9,26 @@ def parse_args() -> argparse.Namespace: - """Parse command line arguments.""" + """Parse command line arguments. + + Generating Chaste C++ code from an SBML file is the default action. Passing ``--copy-base-classes`` + switches to copying the C++ base classes the generated code depends on instead; in that mode no + SBML file is taken and the only other option that applies is ``--output-dir``. + """ parser = argparse.ArgumentParser( prog="chaste-sbml", description="Convert SBML models to Chaste C++ code", ) parser.add_argument("--version", action="version", version="%(prog)s " + __version__) - subparsers = parser.add_subparsers(dest="command", required=True, metavar="command") - - # generate: create Chaste C++ code from an SBML file. - generate = subparsers.add_parser( - "generate", - help="Generate Chaste C++ code from an SBML file", - description="Generate Chaste C++ code from an SBML file", + parser.add_argument("sbml_file", nargs="?", help="The SBML file to convert") + parser.add_argument( + "--copy-base-classes", + action="store_true", + help="Copy the C++ base classes the generated code depends on, instead of generating code", ) - generate.add_argument("sbml_file", help="The SBML file to convert") - generate.add_argument("--output-dir", default=None, help="The directory to place output files in") - generate.add_argument( + parser.add_argument("--output-dir", default=None, help="The directory to place output files in") + parser.add_argument( "--model-type", help="The type of model to generate", choices=["generic", "srn", "cell-cycle"], @@ -34,34 +36,34 @@ def parse_args() -> argparse.Namespace: const="generic", nargs="?", ) - generate.add_argument( + parser.add_argument( "--tests", action=argparse.BooleanOptionalAction, default=True, help="Generate placeholder test files (default: on). Use --no-tests to disable.", ) - generate.add_argument( + parser.add_argument( "--timescale", choices=["ms", "s", "m", "h"], default=None, help="The model's native time unit (milliseconds/seconds/minutes/hours), used to convert " "derivatives to Chaste's hours. Overrides auto-detection from the SBML; omit to auto-detect.", ) - generate.add_argument( + parser.add_argument( "--test-output-dir", default=None, help="The directory to place generated test files in (defaults to --output-dir)", ) - # copy-base-classes: copy the C++ base classes the generated code depends on. - copy_parser = subparsers.add_parser( - "copy-base-classes", - help="Copy the C++ base classes the generated code depends on", - description="Copy the C++ base classes the generated code depends on", - ) - copy_parser.add_argument("--output-dir", default=None, help="The directory to place the base classes in") + args = parser.parse_args() + + if args.copy_base_classes: + if args.sbml_file is not None: + parser.error("--copy-base-classes does not take an SBML file") + elif args.sbml_file is None: + parser.error("an SBML file is required (or pass --copy-base-classes to copy the base classes)") - return parser.parse_args() + return args def process_command_line(args: "argparse.Namespace"): @@ -69,7 +71,7 @@ def process_command_line(args: "argparse.Namespace"): :args: The parsed command line arguments. """ - if args.command == "copy-base-classes": + if args.copy_base_classes: copy_base_classes(args.output_dir) return diff --git a/chaste_sbml/tests/test_cli.py b/chaste_sbml/tests/test_cli.py index 858a5551..832cefc8 100644 --- a/chaste_sbml/tests/test_cli.py +++ b/chaste_sbml/tests/test_cli.py @@ -28,23 +28,23 @@ def test_version_exits_zero(monkeypatch): @pytest.mark.parametrize("model_type", ["generic", "srn", "cell-cycle"]) def test_generate_each_model_type(monkeypatch, tmp_path, model_type): - """generate handles every --model-type and emits the model plus placeholder test.""" - _run(monkeypatch, "generate", str(GOLDBETER), "--model-type", model_type, "--output-dir", str(tmp_path)) + """Generation (the default action) handles every --model-type and emits the model plus test.""" + _run(monkeypatch, str(GOLDBETER), "--model-type", model_type, "--output-dir", str(tmp_path)) assert (tmp_path / "Goldbeter1991SbmlOdeSystem.hpp").is_file() assert (tmp_path / "TestGoldbeter1991Sbml.hpp").is_file() def test_generate_no_tests(monkeypatch, tmp_path): - """generate --no-tests writes the model but no placeholder test.""" - _run(monkeypatch, "generate", str(GOLDBETER), "--output-dir", str(tmp_path), "--no-tests") + """--no-tests writes the model but no placeholder test.""" + _run(monkeypatch, str(GOLDBETER), "--output-dir", str(tmp_path), "--no-tests") assert (tmp_path / "Goldbeter1991SbmlOdeSystem.hpp").is_file() assert not (tmp_path / "TestGoldbeter1991Sbml.hpp").exists() def test_generate_test_output_dir(monkeypatch, tmp_path): - """generate --test-output-dir routes the placeholder test to its own directory.""" + """--test-output-dir routes the placeholder test to its own directory.""" src_dir = tmp_path / "src" test_dir = tmp_path / "test" src_dir.mkdir() @@ -52,7 +52,6 @@ def test_generate_test_output_dir(monkeypatch, tmp_path): _run( monkeypatch, - "generate", str(GOLDBETER), "--output-dir", str(src_dir), @@ -65,7 +64,23 @@ def test_generate_test_output_dir(monkeypatch, tmp_path): def test_copy_base_classes(monkeypatch, tmp_path): - """copy-base-classes copies the C++ base classes into --output-dir.""" - _run(monkeypatch, "copy-base-classes", "--output-dir", str(tmp_path)) + """--copy-base-classes copies the C++ base classes into --output-dir.""" + _run(monkeypatch, "--copy-base-classes", "--output-dir", str(tmp_path)) assert (tmp_path / "AbstractSbmlOdeSystem.hpp").is_file() + + +def test_missing_sbml_file_is_usage_error(monkeypatch): + """No SBML file and no --copy-base-classes exits with a usage error (code 2).""" + with pytest.raises(SystemExit) as exc: + _run(monkeypatch) + + assert exc.value.code == 2 + + +def test_copy_base_classes_with_sbml_file_is_usage_error(monkeypatch): + """--copy-base-classes takes no SBML file; supplying one exits with a usage error (code 2).""" + with pytest.raises(SystemExit) as exc: + _run(monkeypatch, "--copy-base-classes", str(GOLDBETER)) + + assert exc.value.code == 2 diff --git a/chaste_sbml/tests/test_console.py b/chaste_sbml/tests/test_console.py index 2a054868..c15cfce3 100644 --- a/chaste_sbml/tests/test_console.py +++ b/chaste_sbml/tests/test_console.py @@ -12,11 +12,11 @@ def test_help(): - """The top-level help lists both subcommands.""" + """The help documents the default SBML file argument and the copy-base-classes switch.""" output = subprocess.check_output(["chaste-sbml", "-h"]).decode("ascii") - assert "generate" in output - assert "copy-base-classes" in output + assert "sbml_file" in output + assert "--copy-base-classes" in output def test_version(): @@ -28,36 +28,28 @@ def test_version(): assert output == expected -def test_requires_subcommand(): - """Running with no subcommand is a usage error (exit code 2).""" +def test_requires_sbml_file(): + """Running with no SBML file and no --copy-base-classes is a usage error (exit code 2).""" result = subprocess.run(["chaste-sbml"], capture_output=True, text=True) assert result.returncode == 2 -def test_generate_requires_sbml_file(): - """generate without an SBML file is a usage error (exit code 2).""" - result = subprocess.run(["chaste-sbml", "generate"], capture_output=True, text=True) +def test_copy_base_classes_rejects_sbml_file(): + """--copy-base-classes takes no SBML file; supplying one is a usage error (exit 2).""" + result = subprocess.run( + ["chaste-sbml", "--copy-base-classes", "model.xml"], + capture_output=True, + text=True, + ) assert result.returncode == 2 -def test_copy_base_classes_rejects_generation_options(): - """copy-base-classes takes only --output-dir; generation options are usage errors (exit 2).""" - for extra in (["model.xml"], ["--model-type", "srn"]): - result = subprocess.run( - ["chaste-sbml", "copy-base-classes", *extra], - capture_output=True, - text=True, - ) - - assert result.returncode == 2, extra - - def test_copy_base_classes_accepts_output_dir(tmp_path): - """copy-base-classes with --output-dir copies the base classes.""" + """--copy-base-classes with --output-dir copies the base classes.""" result = subprocess.run( - ["chaste-sbml", "copy-base-classes", "--output-dir", str(tmp_path)], + ["chaste-sbml", "--copy-base-classes", "--output-dir", str(tmp_path)], capture_output=True, text=True, ) @@ -67,9 +59,9 @@ def test_copy_base_classes_accepts_output_dir(tmp_path): def test_generate_emits_placeholder_test(tmp_path): - """generate produces a placeholder test file alongside the model by default.""" + """Generation produces a placeholder test file alongside the model by default.""" result = subprocess.run( - ["chaste-sbml", "generate", str(GOLDBETER), "--output-dir", str(tmp_path)], + ["chaste-sbml", str(GOLDBETER), "--output-dir", str(tmp_path)], capture_output=True, text=True, ) @@ -80,9 +72,9 @@ def test_generate_emits_placeholder_test(tmp_path): def test_generate_no_tests_skips_placeholder_test(tmp_path): - """generate --no-tests writes the model but no placeholder test.""" + """--no-tests writes the model but no placeholder test.""" result = subprocess.run( - ["chaste-sbml", "generate", str(GOLDBETER), "--output-dir", str(tmp_path), "--no-tests"], + ["chaste-sbml", str(GOLDBETER), "--output-dir", str(tmp_path), "--no-tests"], capture_output=True, text=True, ) @@ -93,7 +85,7 @@ def test_generate_no_tests_skips_placeholder_test(tmp_path): def test_generate_test_output_dir_routes_test(tmp_path): - """generate --test-output-dir places the placeholder test in its own directory.""" + """--test-output-dir places the placeholder test in its own directory.""" src_dir = tmp_path / "src" test_dir = tmp_path / "test" src_dir.mkdir() @@ -102,7 +94,6 @@ def test_generate_test_output_dir_routes_test(tmp_path): result = subprocess.run( [ "chaste-sbml", - "generate", str(GOLDBETER), "--output-dir", str(src_dir), diff --git a/pyproject.toml b/pyproject.toml index 42cca54c..28f04510 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ chaste-sbml = "chaste_sbml.__main__:main" source = "https://github.com/Chaste/chaste-codegen-sbml" [tool.setuptools.package-data] -# Ship the C++ base classes so `chaste-sbml copy-base-classes` can copy them out +# Ship the C++ base classes so `chaste-sbml --copy-base-classes` can copy them out # of an installed package. (setuptools-scm also tracks these, but declaring them explicitly # keeps them in the wheel regardless of the VCS file finder.) chaste_sbml = [ From 1dc0b0e42c7ced247b23cb71ad370f763b8178e7 Mon Sep 17 00:00:00 2001 From: Kwabena Amponsah Date: Wed, 12 Aug 2026 15:56:08 +0100 Subject: [PATCH 2/2] #39 Reject generation options in copy-base-classes mode Copy mode takes only --output-dir, but the flat parser accepted and silently ignored generation-only options (--model-type, --tests, --timescale, --test-output-dir). Give those options default=SUPPRESS so a missing attribute reliably means "not supplied", then report a usage error in copy mode when an SBML file or any of them is passed, and fill in the real defaults when generating. Restore the rejection test and document the behaviour. Co-Authored-By: Claude Opus 4.8 --- README.md | 3 ++- chaste_sbml/__main__.py | 41 ++++++++++++++++++++++++------- chaste_sbml/tests/test_cli.py | 12 +++++++++ chaste_sbml/tests/test_console.py | 25 ++++++++++++------- 4 files changed, 62 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 28b13716..b919ecea 100644 --- a/README.md +++ b/README.md @@ -116,4 +116,5 @@ they match the installed version of `chaste-sbml`: chaste-sbml --copy-base-classes --output-dir src/ ``` -In this mode no SBML file is taken and the only other option that applies is `--output-dir`. +In this mode only `--output-dir` applies; passing an SBML file or a generation option (such as +`--model-type` or `--no-tests`) is a usage error. diff --git a/chaste_sbml/__main__.py b/chaste_sbml/__main__.py index 845eaf94..e3bb0863 100644 --- a/chaste_sbml/__main__.py +++ b/chaste_sbml/__main__.py @@ -7,6 +7,17 @@ from ._version import __version__ +# Options that apply only when generating, mapped to the default each takes. In copy mode none may +# be supplied (only --output-dir applies), so they use ``argparse.SUPPRESS`` as their default: the +# attribute is absent unless the option was passed, which lets copy mode reject an explicitly-supplied +# option while generation fills in the real default below. +_GENERATION_DEFAULTS = { + "model_type": "generic", + "tests": True, + "timescale": None, + "test_output_dir": None, +} + def parse_args() -> argparse.Namespace: """Parse command line arguments. @@ -30,38 +41,50 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--output-dir", default=None, help="The directory to place output files in") parser.add_argument( "--model-type", - help="The type of model to generate", + help="The type of model to generate (default: generic)", choices=["generic", "srn", "cell-cycle"], - default="generic", + default=argparse.SUPPRESS, const="generic", nargs="?", ) parser.add_argument( "--tests", action=argparse.BooleanOptionalAction, - default=True, + default=argparse.SUPPRESS, help="Generate placeholder test files (default: on). Use --no-tests to disable.", ) parser.add_argument( "--timescale", choices=["ms", "s", "m", "h"], - default=None, + default=argparse.SUPPRESS, help="The model's native time unit (milliseconds/seconds/minutes/hours), used to convert " "derivatives to Chaste's hours. Overrides auto-detection from the SBML; omit to auto-detect.", ) parser.add_argument( "--test-output-dir", - default=None, + default=argparse.SUPPRESS, help="The directory to place generated test files in (defaults to --output-dir)", ) args = parser.parse_args() + # Options are stored only when supplied (default=SUPPRESS), so a missing attribute means unset. + supplied_generation_opts = [name for name in _GENERATION_DEFAULTS if hasattr(args, name)] + if args.copy_base_classes: - if args.sbml_file is not None: - parser.error("--copy-base-classes does not take an SBML file") - elif args.sbml_file is None: - parser.error("an SBML file is required (or pass --copy-base-classes to copy the base classes)") + # Copy mode takes only --output-dir; reject the SBML file and any generation-only option + # rather than silently ignoring them. + rejected = ["an SBML file"] if args.sbml_file is not None else [] + rejected += [f"--{name.replace('_', '-')}" for name in supplied_generation_opts] + if rejected: + parser.error("--copy-base-classes takes only --output-dir, not: " + ", ".join(rejected)) + else: + if args.sbml_file is None: + parser.error("an SBML file is required (or pass --copy-base-classes to copy the base classes)") + # Fill in the real default for every generation option that was not supplied. + for name, default in _GENERATION_DEFAULTS.items(): + if not hasattr(args, name): + setattr(args, name, default) return args diff --git a/chaste_sbml/tests/test_cli.py b/chaste_sbml/tests/test_cli.py index 832cefc8..3a919f36 100644 --- a/chaste_sbml/tests/test_cli.py +++ b/chaste_sbml/tests/test_cli.py @@ -84,3 +84,15 @@ def test_copy_base_classes_with_sbml_file_is_usage_error(monkeypatch): _run(monkeypatch, "--copy-base-classes", str(GOLDBETER)) assert exc.value.code == 2 + + +@pytest.mark.parametrize( + "extra", + [["--model-type", "srn"], ["--no-tests"], ["--timescale", "s"], ["--test-output-dir", "test/"]], +) +def test_copy_base_classes_rejects_generation_options(monkeypatch, extra): + """A generation-only option in copy mode exits with a usage error (code 2), not silent ignore.""" + with pytest.raises(SystemExit) as exc: + _run(monkeypatch, "--copy-base-classes", *extra) + + assert exc.value.code == 2 diff --git a/chaste_sbml/tests/test_console.py b/chaste_sbml/tests/test_console.py index c15cfce3..b8536816 100644 --- a/chaste_sbml/tests/test_console.py +++ b/chaste_sbml/tests/test_console.py @@ -35,15 +35,22 @@ def test_requires_sbml_file(): assert result.returncode == 2 -def test_copy_base_classes_rejects_sbml_file(): - """--copy-base-classes takes no SBML file; supplying one is a usage error (exit 2).""" - result = subprocess.run( - ["chaste-sbml", "--copy-base-classes", "model.xml"], - capture_output=True, - text=True, - ) - - assert result.returncode == 2 +def test_copy_base_classes_rejects_generation_options(): + """--copy-base-classes takes only --output-dir; an SBML file or generation option is a usage error.""" + for extra in ( + ["model.xml"], + ["--model-type", "srn"], + ["--no-tests"], + ["--timescale", "s"], + ["--test-output-dir", "test/"], + ): + result = subprocess.run( + ["chaste-sbml", "--copy-base-classes", *extra], + capture_output=True, + text=True, + ) + + assert result.returncode == 2, extra def test_copy_base_classes_accepts_output_dir(tmp_path):