Skip to content

Commit

Permalink
Atemerev/opts config (#115)
Browse files Browse the repository at this point in the history
## 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>
  • Loading branch information
atemerev and jorblancoa authored Jan 31, 2024
1 parent 1f277d8 commit 35d07fd
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 18 deletions.
25 changes: 8 additions & 17 deletions init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
27 changes: 27 additions & 0 deletions neurodamus/utils/cli.py
Original file line number Diff line number Diff line change
@@ -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 <config_file.json> ...")
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
2 changes: 1 addition & 1 deletion tests/unit/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest
import sys
import unittest
import unittest.mock


class MockParallelExec:
Expand Down
51 changes: 51 additions & 0 deletions tests/unit/test_init_py_config.py
Original file line number Diff line number Diff line change
@@ -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'])

0 comments on commit 35d07fd

Please sign in to comment.