From 35d07fd86f2d06ef4f8750fbb1df27c0949b4c4d Mon Sep 17 00:00:00 2001 From: Alexander Temerev Date: Wed, 31 Jan 2024 16:01:58 +0100 Subject: [PATCH] Atemerev/opts config (#115) ## Context Options handling by `init.py`: `special` only supports positional options. Attempting to specify a config file as a positional option led to confusing error messages. Now, using positional options in init.py lead to an error message explaining that only keyword options are supported. --configFile can be used anywhere on the command line after init.py, the rest of the options are passed correctly to `neurodamus.commands`. ## Scope Init.py is affected, no new dependencies added. Handling arguments is extracted to a function for unit testing. ## Testing tests/unit/test_init_py_config.py ## Review * [X] PR description is complete * [X] Coding style (imports, function length, New functions, classes or files) are good * [X] Unit/Scientific test added * [N/A] Updated Readme, in-code, developer documentation --------- Co-authored-by: Jorge Blanco Alonso <41900536+jorblancoa@users.noreply.github.com> --- init.py | 25 +++++---------- neurodamus/utils/cli.py | 27 ++++++++++++++++ tests/unit/conftest.py | 2 +- tests/unit/test_init_py_config.py | 51 +++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 neurodamus/utils/cli.py create mode 100644 tests/unit/test_init_py_config.py diff --git a/init.py b/init.py index 2b35aa0a..0ea3075c 100644 --- a/init.py +++ b/init.py @@ -6,27 +6,18 @@ """ import sys from neurodamus import commands +from neurodamus.utils.cli import extract_arguments from neuron import h +import logging def main(): - """Get the options for neurodamus and launch it. - - We can't use positional arguments with special so we look for - --configFile=FILE, which defaults to simulation_config.json - """ - first_argument_pos = 1 - config_file = "simulation_config.json" - - for i, arg in enumerate(sys.argv): - if arg.endswith("init.py"): - first_argument_pos = i + 1 - elif arg.startswith("--configFile="): - config_file = arg.split('=')[1] - first_argument_pos = i + 1 - break - - args = [config_file] + sys.argv[first_argument_pos:] + args = [] + try: + args = extract_arguments(sys.argv) + except ValueError as err: + logging.error(err) + return 1 return commands.neurodamus(args) diff --git a/neurodamus/utils/cli.py b/neurodamus/utils/cli.py new file mode 100644 index 00000000..336c083d --- /dev/null +++ b/neurodamus/utils/cli.py @@ -0,0 +1,27 @@ +def extract_arguments(args): + """Get the options for neurodamus and launch it. + + We can't use positional arguments with special so we look for + --configFile=FILE, which defaults to simulation_config.json + """ + first_argument_pos = 1 + init_py_reached = False + config_file = "simulation_config.json" + for i, arg in enumerate(args): + if not init_py_reached: + if arg.endswith("init.py"): + first_argument_pos = i + 1 + init_py_reached = True + elif arg.startswith("--configFile="): + raise ValueError("Usage: ... init.py --configFile ...") + continue + + elif not arg.startswith("-"): + raise ValueError("Positional arguments are not supported by init.py. " + f"Found positional argument: '{arg}'") + elif arg.startswith("--configFile="): + config_file = arg.split('=')[1] + + result_args = ([config_file] + + [x for x in args[first_argument_pos:] if not x.startswith("--configFile=")]) + return result_args diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 1354ff4b..fab0d845 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,6 +1,6 @@ import pytest import sys -import unittest +import unittest.mock class MockParallelExec: diff --git a/tests/unit/test_init_py_config.py b/tests/unit/test_init_py_config.py new file mode 100644 index 00000000..185e3cfd --- /dev/null +++ b/tests/unit/test_init_py_config.py @@ -0,0 +1,51 @@ +import pytest + +from neurodamus.utils.cli import extract_arguments + +_default_config_file = 'simulation_config.json' + + +def test_init_empty(): + args = extract_arguments(['some/dir/init.py']) + assert len(args) == 1 + assert args[0] == _default_config_file + + +def test_init_non_positional(): + with pytest.raises(ValueError): + extract_arguments(['init.py', 'simulation_config.json']) + + +def test_init_config_only(): + args = extract_arguments(['another/dir/init.py', + '--configFile=conf/my_config.json']) + assert len(args) == 1 + assert args[0] == 'conf/my_config.json' + + +def test_init_pass_options(): + args = extract_arguments(['another/dir/init.py', '--foo=bar', + '--configFile=conf/my_config.json', + '-v', '--baz=qux']) + assert len(args) == 4 + assert args[0] == 'conf/my_config.json' + assert args[1] == '--foo=bar' + assert args[2] == '-v' + assert args[3] == '--baz=qux' + + +def test_init_first_args(): + args = extract_arguments(['dplace', 'special' 'another/dir/init.py', '--foo=bar', + '--configFile=conf/my_config.json', + '-v', '--baz=qux']) + assert len(args) == 4 + assert args[0] == 'conf/my_config.json' + assert args[1] == '--foo=bar' + assert args[2] == '-v' + assert args[3] == '--baz=qux' + + +def test_init_early_config_file(): + with pytest.raises(ValueError): + extract_arguments(['dplace', 'special', '--configFile=my_config.json', '--some=pre', + 'etc', 'dir/init.py', '--foo=bar', '-v'])