From 4c3b488f73f246ead13516e1de2cefcdb7f738da Mon Sep 17 00:00:00 2001 From: "Ankur Sinha (Ankur Sinha Gmail)" Date: Mon, 5 Aug 2024 16:45:51 +0100 Subject: [PATCH 1/6] feat(io): add generic method to check for file extensions --- pyneuroml/errors.py | 6 ++++ pyneuroml/io.py | 85 ++++++++++++++++++++++++++++++--------------- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/pyneuroml/errors.py b/pyneuroml/errors.py index e0d5b0d1..515ef85f 100644 --- a/pyneuroml/errors.py +++ b/pyneuroml/errors.py @@ -10,3 +10,9 @@ FILE_NOT_FOUND_ERR = 13 ARGUMENT_ERR = 14 UNKNOWN_ERR = 15 + + +class NMLFileTypeError(RuntimeError): + """A custom file type error to use for failing file checks.""" + + pass diff --git a/pyneuroml/io.py b/pyneuroml/io.py index 55acb4b4..1b001fd1 100644 --- a/pyneuroml/io.py +++ b/pyneuroml/io.py @@ -15,12 +15,12 @@ import typing from typing import Optional +import lems.model.model as lems_model import neuroml.loaders as loaders import neuroml.writers as writers from neuroml import NeuroMLDocument -import lems.model.model as lems_model -from pyneuroml.errors import FILE_NOT_FOUND_ERR +from pyneuroml.errors import FILE_NOT_FOUND_ERR, NMLFileTypeError from pyneuroml.validators import validate_neuroml2 logger = logging.getLogger(__name__) @@ -130,12 +130,12 @@ def read_neuroml2_file( if fix_external_morphs_biophys: from neuroml.utils import fix_external_morphs_biophys_in_cell + fix_external_morphs_biophys_in_cell(nml2_doc) return nml2_doc - def write_neuroml2_file( nml2_doc: NeuroMLDocument, nml2_file_name: str, @@ -235,21 +235,21 @@ def confirm_neuroml_file(filename: str) -> None: :param filename: Names of files to check :type filename: str """ - # print('Checking file: %s'%filename) - # Some conditions to check if a LEMS file was entered - # TODO: Ideally we'd like to check the root node: checking file extensions is brittle - confirm_file_exists(filename) - if filename.startswith("LEMS_"): - logger.warning( - textwrap.dedent( - """ - ************************************************************************************* - ** Warning, you may be trying to use a LEMS XML file (containing etc.) - ** for a pyNeuroML option when a NeuroML2 file is required... - ************************************************************************************* - """ - ) - ) + error_string = textwrap.dedent( + """ + ************************************************************************************* + ** You may be trying to use a LEMS XML file (containing etc.) + ** for a pyNeuroML option when a NeuroML2 file is required. + ************************************************************************************* + """ + ) + + try: + confirm_file_type(filename, ["nml"]) + except NMLFileTypeError as e: + if filename.startswith("LEMS_"): + logger.warning(error_string) + raise e def confirm_lems_file(filename: str) -> None: @@ -262,15 +262,44 @@ def confirm_lems_file(filename: str) -> None: # print('Checking file: %s'%filename) # Some conditions to check if a LEMS file was entered # TODO: Ideally we'd like to check the root node: checking file extensions is brittle + error_string = textwrap.dedent( + """ + ************************************************************************************* + ** You may be trying to use a NeuroML2 file for a pyNeuroML option + ** when a LEMS XML file (containing etc.) is required. + ************************************************************************************* + """ + ) + try: + confirm_file_type(filename, ["xml"]) + except NMLFileTypeError as e: + if filename.endswith("nml"): + logger.warning(error_string) + raise e + + +def confirm_file_type( + filename: str, file_exts: typing.List[str], error_str: typing.Optional[str] = None +) -> None: + """Confirm that a file exists and has the necessary extension + + :param filename: filename to confirm + :type filename: str + :param file_exts: list of valid file extensions, without the leading dot + :type file_exts: list of strings + :param error_str: an optional error string to print along with the thrown + exception + :type error_str: string (optional) + + :raises NMLFileTypeError: if file does not have one of the provided extensions + + """ confirm_file_exists(filename) - if filename.endswith("nml"): - logger.warning( - textwrap.dedent( - """ - ************************************************************************************* - ** Warning, you may be trying to use a NeuroML2 file for a pyNeuroML option - ** when a LEMS XML file (containing etc.) is required... - ************************************************************************************* - """ - ) + filename_ext = filename.split(".")[-1] + if filename_ext not in file_exts: + error_string = ( + f"Expected filetypes: {', '.join(file_exts)} (got {filename_ext})" ) + if error_str is not None: + error_string += "\n" + error_str + raise NMLFileTypeError(error_string) From 9c7dde93f0fb493888a2b9488f1b572c0b4d0985 Mon Sep 17 00:00:00 2001 From: "Ankur Sinha (Ankur Sinha Gmail)" Date: Tue, 6 Aug 2024 09:14:49 +0100 Subject: [PATCH 2/6] feat(io): note what standard for expected file type is --- pyneuroml/io.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pyneuroml/io.py b/pyneuroml/io.py index 1b001fd1..aa44a2c5 100644 --- a/pyneuroml/io.py +++ b/pyneuroml/io.py @@ -27,6 +27,15 @@ logger.setLevel(logging.INFO) +# extension: standard +pynml_file_type_dict = { + "xml": "LEMS", + "nml": "NeuroML", + "sedml": "SED-ML", + "sbml": "SBML", +} + + def read_neuroml2_file( nml2_file_name: str, include_includes: bool = False, @@ -296,9 +305,10 @@ def confirm_file_type( """ confirm_file_exists(filename) filename_ext = filename.split(".")[-1] + file_types = [f"{x} ({pynml_file_type_dict[x]})" for x in file_exts] if filename_ext not in file_exts: error_string = ( - f"Expected filetypes: {', '.join(file_exts)} (got {filename_ext})" + f"Expected file extension(s): {', '.join(file_types)}; got {filename_ext}" ) if error_str is not None: error_string += "\n" + error_str From e4fb698dd8e056c8bfbb9a37e3f7deed0d1fa104 Mon Sep 17 00:00:00 2001 From: "Ankur Sinha (Ankur Sinha Gmail)" Date: Tue, 6 Aug 2024 10:06:01 +0100 Subject: [PATCH 3/6] feat(io): make function optionally exit instead of throwing exception --- pyneuroml/io.py | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/pyneuroml/io.py b/pyneuroml/io.py index aa44a2c5..295afd26 100644 --- a/pyneuroml/io.py +++ b/pyneuroml/io.py @@ -20,7 +20,7 @@ import neuroml.writers as writers from neuroml import NeuroMLDocument -from pyneuroml.errors import FILE_NOT_FOUND_ERR, NMLFileTypeError +from pyneuroml.errors import ARGUMENT_ERR, FILE_NOT_FOUND_ERR, NMLFileTypeError from pyneuroml.validators import validate_neuroml2 logger = logging.getLogger(__name__) @@ -237,12 +237,14 @@ def confirm_file_exists(filename: str) -> None: sys.exit(FILE_NOT_FOUND_ERR) -def confirm_neuroml_file(filename: str) -> None: +def confirm_neuroml_file(filename: str, sys_error: bool = False) -> None: """Confirm that file exists and is a NeuroML file before proceeding with processing. :param filename: Names of files to check :type filename: str + :param sys_error: toggle whether function should exit or raise exception + :type sys_error: bool """ error_string = textwrap.dedent( """ @@ -258,15 +260,21 @@ def confirm_neuroml_file(filename: str) -> None: except NMLFileTypeError as e: if filename.startswith("LEMS_"): logger.warning(error_string) - raise e + if sys_error is True: + logger.error(e) + sys.exit(ARGUMENT_ERR) + else: + raise e -def confirm_lems_file(filename: str) -> None: +def confirm_lems_file(filename: str, sys_error: bool = False) -> None: """Confirm that file exists and is a LEMS file before proceeding with processing. :param filename: Names of files to check :type filename: list of strings + :param sys_error: toggle whether function should exit or raise exception + :type sys_error: bool """ # print('Checking file: %s'%filename) # Some conditions to check if a LEMS file was entered @@ -284,11 +292,18 @@ def confirm_lems_file(filename: str) -> None: except NMLFileTypeError as e: if filename.endswith("nml"): logger.warning(error_string) - raise e + if sys_error is True: + logger.error(e) + sys.exit(ARGUMENT_ERR) + else: + raise e def confirm_file_type( - filename: str, file_exts: typing.List[str], error_str: typing.Optional[str] = None + filename: str, + file_exts: typing.List[str], + error_str: typing.Optional[str] = None, + sys_error: bool = False, ) -> None: """Confirm that a file exists and has the necessary extension @@ -312,4 +327,8 @@ def confirm_file_type( ) if error_str is not None: error_string += "\n" + error_str - raise NMLFileTypeError(error_string) + if sys_error is True: + logger.error(error_string) + sys.exit(ARGUMENT_ERR) + else: + raise NMLFileTypeError(error_string) From fc829183118f57d6f858fe59911bedc4e6a816e9 Mon Sep 17 00:00:00 2001 From: "Ankur Sinha (Ankur Sinha Gmail)" Date: Tue, 6 Aug 2024 10:16:44 +0100 Subject: [PATCH 4/6] feat(pynml): improve checks on file types being passed --- pyneuroml/pynml.py | 106 ++++++++++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 35 deletions(-) diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index 385870d4..9966390e 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -561,9 +561,13 @@ def _evaluate_arguments(args): pre_args = "" post_args = "" exit_on_fail = True + # type of files being passed to the command + # by default, lems + file_types = ["xml"] # Deal with the SBML validation option which doesn't call run_jneuroml if args.validate_sbml or args.validate_sbml_units: + file_types = ["sbml"] try: from pyneuroml.sbml import validate_sbml_files except Exception: @@ -597,6 +601,7 @@ def _evaluate_arguments(args): # Deal with the SEDML validation option which doesn't call run_jneuroml if args.validate_sedml: + file_types = ["sedml"] try: from pyneuroml.sedml import validate_sedml_files except Exception: @@ -623,6 +628,7 @@ def _evaluate_arguments(args): # Deal with the -run-tellurium option which doesn't call run_jneuroml if args.run_tellurium is not None: + file_types = ["sedml"] try: from pyneuroml.tellurium import run_from_sedml_file except Exception: @@ -654,16 +660,21 @@ def _evaluate_arguments(args): # TODO: handle these better if args.sbml_import or args.sbml_import_units or args.vhdl: if args.sbml_import: + file_types = ["sbml"] pre_args = "-sbml-import" f = args.sbml_import[0] post_args = " ".join(args.sbml_import[1:]) + confirm_file_type(f, file_types, sys_error=True) elif args.sbml_import_units: + file_types = ["sbml"] pre_args = "-smbl-import-units" f = args.sbml_import_units[0] post_args = " ".join(args.sbml_import_units[1:]) + confirm_file_type(f, file_types, sys_error=True) elif args.vhdl: + file_types = ["xml"] f = args.vhdl[1] - confirm_lems_file(f) + confirm_lems_file(f, True) post_args = "-vhdl %s" % args.vhdl[0] run_jneuroml( @@ -678,21 +689,25 @@ def _evaluate_arguments(args): # Process bits that process the file list provided as the shared option if len(args.input_files) == 0: - logger.critical("Please specify NeuroML/LEMS files to process") + logger.error( + "Please specify files and options to process. Run `pynml -h` to see usage help text." + ) return + # some commands can be run on lists of files run_multi = False + # for commands to be run on each file individually for f in args.input_files: if args.nogui: post_args = "-nogui" if args.sedml: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-sedml" elif args.neuron is not None: # Note: either a lems file or nml2 file is allowed here... - confirm_file_exists(f) + file_types = ["xml", "nml"] num_neuron_args = len(args.neuron) if num_neuron_args < 0 or num_neuron_args > 4: @@ -707,7 +722,7 @@ def _evaluate_arguments(args): elif args.netpyne is not None: # Note: either a lems file or nml2 file is allowed here... - confirm_file_exists(f) + file_types = ["xml", "nml"] num_netpyne_args = len(args.netpyne) @@ -722,7 +737,7 @@ def _evaluate_arguments(args): post_args = "-netpyne %s" % " ".join(other_args) elif args.eden is not None: - confirm_lems_file(f) + file_types = ["xml"] num_eden_args = len(args.eden) @@ -736,53 +751,60 @@ def _evaluate_arguments(args): other_args = [(a if a != "-eden" else "") for a in args.eden] post_args = "-eden %s" % " ".join(other_args) - elif args.svg: - confirm_neuroml_file(f) - post_args = "-svg" - elif args.png: - confirm_neuroml_file(f) - post_args = "-png" elif args.dlems: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-dlems" elif args.vertex: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-vertex" elif args.xpp: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-xpp" elif args.dnsim: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-dnsim" elif args.brian: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-brian" elif args.brian2: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-brian2" elif args.moose: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-moose" elif args.sbml: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-sbml" elif args.sbml_sedml: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-sbml-sedml" elif args.matlab: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-matlab" elif args.cvode: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-cvode" elif args.nineml: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-nineml" elif args.spineml: - confirm_lems_file(f) + file_types = ["xml"] post_args = "-spineml" + elif args.lems_graph: + file_types = ["xml"] + pre_args = "" + post_args = "-lems-graph" + exit_on_fail = True + elif args.svg: + file_types = ["nml"] + post_args = "-svg" + elif args.png: + file_types = ["nml"] + post_args = "-png" elif args.graph: - confirm_neuroml_file(f) + # not using jneuroml + file_types = ["nml"] + confirm_neuroml_file(f, True) from neuromllite.GraphVizHandler import engines engine = "dot" @@ -818,13 +840,8 @@ def _evaluate_arguments(args): generate_nmlgraph(f, level, engine) sys.exit(0) - elif args.lems_graph: - confirm_lems_file(f) - pre_args = "" - post_args = "-lems-graph" - exit_on_fail = True elif args.matrix: - confirm_neuroml_file(f) + confirm_neuroml_file(f, True) from neuromllite.MatrixHandler import MatrixHandler level = int(args.matrix[0]) @@ -845,27 +862,36 @@ def _evaluate_arguments(args): exit(0) elif args.validate: - confirm_neuroml_file(f) + file_types = ["nml"] pre_args = "-validate" exit_on_fail = True run_multi = True elif args.validatev1: - confirm_neuroml_file(f) + file_types = ["nml"] pre_args = "-validatev1" exit_on_fail = True run_multi = True elif args.swc: convert_count = 0 + file_types = ["nml"] for f in args.input_files: - confirm_neuroml_file(f) + confirm_neuroml_file(f, True) logger.info(f"Trying to convert {f} to swc format...") convert_count += 1 if convert_to_swc(f) else 0 logger.info(f"Converted {convert_count} file(s) to swc format") sys.exit(0) if run_multi is False: + # check that the right file type has been passed to jNeuroML + if file_types == ["xml"]: + confirm_lems_file(f, True) + elif file_types == ["nml"]: + confirm_neuroml_file(f, True) + else: + confirm_file_type(f, file_types, sys_error=True) + run_jneuroml( pre_args, f, @@ -873,7 +899,17 @@ def _evaluate_arguments(args): max_memory=args.java_max_memory, exit_on_fail=exit_on_fail, ) + if run_multi: + for f in args.input_files: + # check that the right file type has been passed to jNeuroML + if file_types == ["xml"]: + confirm_lems_file(f, True) + elif file_types == ["nml"]: + confirm_neuroml_file(f, True) + else: + confirm_file_type(f, file_types, sys_error=True) + run_jneuroml( pre_args, " ".join(args.input_files), From 71d8ae8595f0e6abe946460f8f7e2f07d9986609 Mon Sep 17 00:00:00 2001 From: "Ankur Sinha (Ankur Sinha Gmail)" Date: Tue, 6 Aug 2024 10:25:59 +0100 Subject: [PATCH 5/6] feat(pynml): check sbml/sedml file types also --- pyneuroml/pynml.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index 9966390e..d6f6816d 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -578,6 +578,10 @@ def _evaluate_arguments(args): logger.critical("No input files specified") sys.exit(ARGUMENT_ERR) + # check for correct file types + for f in args.input_files: + confirm_file_type(f, file_types, sys_error=True) + if args.validate_sbml_units: # A failed unit consistency check generates an error strict_units = True @@ -612,6 +616,10 @@ def _evaluate_arguments(args): logger.critical("No input files specified") sys.exit(ARGUMENT_ERR) + # check for correct file types + for f in args.input_files: + confirm_file_type(f, file_types, sys_error=True) + try: result = validate_sedml_files(args.input_files) except Exception as e: @@ -639,6 +647,10 @@ def _evaluate_arguments(args): logger.critical("No input files specified") sys.exit(ARGUMENT_ERR) + # check for correct file types + for f in args.input_files: + confirm_file_type(f, file_types, sys_error=True) + try: if len(args.input_files) == 1: sedml_file = [args.input_files[0]] From d3e3984e6bdd6eb5ea5ff47db27337c18349a4a9 Mon Sep 17 00:00:00 2001 From: "Ankur Sinha (Ankur Sinha Gmail)" Date: Tue, 6 Aug 2024 10:50:51 +0100 Subject: [PATCH 6/6] test: add tests to check pynml fails when wrong files are provided Fixes #406 --- test-ghactions.sh | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test-ghactions.sh b/test-ghactions.sh index 1c20e5d1..11b9f47b 100755 --- a/test-ghactions.sh +++ b/test-ghactions.sh @@ -1,9 +1,26 @@ +# note that set -e is flimsy +# http://mywiki.wooledge.org/BashFAQ/105 set -e # CI already installs package and all optional dependencies, so this is redundant. # But we keep it to allow easy local testing. pip install .[dev] +# required to test commands that should fail +# need this because other constructs don't work: +# - command || or && does not check that command has failed +# - ! command does not make script error even with set -e because this is considered to be handled +check_should_fail() { + if [ $? -ne 0 ]; then + echo "TEST PASSED: Command failed as expected." + return 0 + else + echo "TEST ERROR: Command should have failed!" + exit 1 + fi +} + + echo echo "################################################" echo "## Testing all CLI tools" @@ -101,6 +118,28 @@ pynml LEMS_NML2_Ex9_FN.xml -spineml pynml LEMS_NML2_Ex9_FN.xml -sbml +echo +echo "################################################" +echo "## Test wrong file types" + +set +e + +# these should fail, but we need to check them explicity because constructs +# like `! command` are not recognised by `set -e` +pynml NML2_SingleCompHHCell.nml ; check_should_fail +pynml NML2_SingleCompHHCell.nml -dlems ; check_should_fail +pynml NML2_SingleCompHHCell.nml -brian ; check_should_fail +pynml NML2_SingleCompHHCell.nml -cvode ; check_should_fail +pynml NML2_SingleCompHHCell.nml -sbml ; check_should_fail +pynml NML2_SingleCompHHCell.nml -matlab ; check_should_fail +pynml -validate LEMS_NML2_Ex9_FN.xml ; check_should_fail +pynml LEMS_NML2_Ex9_FN.xml -swc ; check_should_fail +pynml LEMS_NML2_Ex9_FN.xml -png ; check_should_fail +pynml -validate test_data/valid_doc.sbml ; check_should_fail +pynml LEMS_NML2_Ex9_FN.xml -validate-sbml ; check_should_fail + +set -e + echo echo "################################################" echo "## Simple SBML validation example"