From 9bca6d44e6589ede37da968efc579df6eaaad57b Mon Sep 17 00:00:00 2001 From: Athanaseus Javas Ramaila Date: Tue, 15 Sep 2026 20:50:48 +0200 Subject: [PATCH] Give source-finder's options to its own parser only argparse classifies every token against the main parser before dispatching, and single-dash options match by prefix there even with allow_abbrev=False. So source-finder -t matched -thresh, -tol, -title and -title-size and failed as ambiguous. parse_args() now hands everything after the subcommand to its parser, over the main parser's defaults. --- aimfast/aimfast.py | 27 +++++++++- aimfast/tests/test_cli_subcommand.py | 80 ++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 aimfast/tests/test_cli_subcommand.py diff --git a/aimfast/aimfast.py b/aimfast/aimfast.py index 485b31c..69bcb35 100644 --- a/aimfast/aimfast.py +++ b/aimfast/aimfast.py @@ -6122,6 +6122,30 @@ def get_argparser(): return parser +def parse_args(argv=None): + """Parse the command line, giving a subcommand's arguments only to its own parser. + + argparse classifies every token against the main parser before dispatching, and + single-dash options match by prefix there regardless of allow_abbrev. So + source-finder's -t matched -thresh, -tol, -title and -title-size and failed as + ambiguous, and its -r broke as soon as a second "-r*" option existed. Parsing the + subcommand's tokens with its own parser avoids the whole class. The result is laid + over the main parser's defaults, which the rest of main() still reads. + """ + argv = list(sys.argv[1:] if argv is None else argv) + parser = get_argparser() + subparsers = next( + (a.choices for a in parser._actions if isinstance(a, argparse._SubParsersAction)), {} + ) + position = next((i for i, token in enumerate(argv) if token in subparsers), None) + if position is None: + return parser.parse_args(argv) + args = parser.parse_args(argv[:position]) + subparsers[argv[position]].parse_args(argv[position + 1:], namespace=args) + args.subcommand = argv[position] + return args + + def main(): """Main function.""" LOGGER.info("Welcome to AIMfast") @@ -6129,8 +6153,7 @@ def main(): _command = " ".join(sys.argv) LOGGER.info(f"Command: {_command}") output_dict = dict() - parser = get_argparser() - args = parser.parse_args() + args = parse_args() # Print default args LOGGER.info(" ".join(f"{k}={v}" for k, v in vars(args).items())) DECIMALS = args.deci diff --git a/aimfast/tests/test_cli_subcommand.py b/aimfast/tests/test_cli_subcommand.py new file mode 100644 index 0000000..ec74cf0 --- /dev/null +++ b/aimfast/tests/test_cli_subcommand.py @@ -0,0 +1,80 @@ +"""A subcommand's options must reach its own parser untouched. + +argparse classifies every token against the main parser before dispatching to a +subcommand, and single-dash options match by prefix there (allow_abbrev does not stop +that for "-x" style options). source-finder's -t matched -thresh, -tol, -title and +-title-size and failed as ambiguous; its -r broke once a second "-r*" option existed. +""" +import shutil +import subprocess + +import pytest + +from aimfast import aimfast + + +def _source_finder_parser(): + parser = aimfast.get_argparser() + group = next(a for a in parser._actions if a.__class__.__name__ == "_SubParsersAction") + return group.choices["source-finder"] + + +def _value_for(action): + if action.choices: + return str(next(iter(action.choices))) + if action.type in (int, float): + return "1" + return "somevalue" + + +class TestSourceFinderOptions(object): + def test_dash_t_sets_the_threshold(self): + args = aimfast.parse_args(["source-finder", "-t", "7"]) + assert float(args.sf_thresh) == 7 + assert args.subcommand == "source-finder" + + def test_dash_r_sets_the_image(self): + assert aimfast.parse_args(["source-finder", "-r", "img.fits"]).sf_restored == "img.fits" + + def test_every_option_that_takes_a_value_parses(self): + """Guards the whole class, not only the options that happened to break.""" + checked = 0 + for action in _source_finder_parser()._actions: + if not action.option_strings or action.nargs == 0: + continue + value = _value_for(action) + expected = action.type(value) if action.type in (int, float) else value + for option in action.option_strings: + got = getattr(aimfast.parse_args(["source-finder", option, value]), action.dest) + assert got == expected, f"{option} gave {got!r}" + checked += 1 + assert checked > 5 + + def test_main_parser_defaults_are_still_present(self): + """main() reads main-parser attributes even on the subcommand path.""" + args = aimfast.parse_args(["source-finder", "-t", "7"]) + defaults = aimfast.get_argparser().parse_args([]) + for name, value in vars(defaults).items(): + if name != "subcommand": + assert getattr(args, name) == value, name + + def test_main_options_before_the_subcommand_still_apply(self): + args = aimfast.parse_args(["--outfile", "x.json", "source-finder", "-t", "7"]) + assert args.outfile == "x.json" and float(args.sf_thresh) == 7 + + +class TestWithoutSubcommand(object): + def test_main_options_parse_as_before(self): + args = aimfast.parse_args(["--compare-residuals", "a.fits", "b.fits"]) + assert args.noise == [["a.fits", "b.fits"]] + assert args.subcommand is None + + +def test_console_script_accepts_dash_t(): + exe = shutil.which("aimfast") + if exe is None: + pytest.skip("aimfast console script not on PATH") + proc = subprocess.run([exe, "source-finder", "-r", "foo.fits", "-t", "10", "--help"], + capture_output=True, text=True) + assert "ambiguous option" not in proc.stderr, proc.stderr + assert proc.returncode == 0, proc.stderr