From c69993fb69e38cda83a0d2cf254feda87ae888ef Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Thu, 26 Oct 2023 17:39:56 +0000 Subject: [PATCH 01/14] initial attempt at a validate-sbml option for pynml.py --- pyneuroml/pynml.py | 11 +++++ pyneuroml/sbml/__init__.py | 64 ++++++++++++++++++++++++++++++ tests/sbml/test_data/test_doc.sbml | 42 ++++++++++++++++++++ tests/sbml/test_sbml.py | 6 +++ 4 files changed, 123 insertions(+) create mode 100644 pyneuroml/sbml/__init__.py create mode 100644 tests/sbml/test_data/test_doc.sbml create mode 100755 tests/sbml/test_sbml.py diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index df1f45bb1..da1c7f5ea 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -345,6 +345,11 @@ def parse_arguments(): action="store_true", help=("(Via jNeuroML) Validate NeuroML file(s) against the\n" "v1.8.1 Schema"), ) + mut_exc_opts.add_argument( + "-validate-sbml", + action="store_true", + help=("Validate SBML file(s)"), + ) return parser.parse_args() @@ -2111,6 +2116,12 @@ def evaluate_arguments(args): post_args = "" exit_on_fail = True + # Deal with the SBML validation option which doesn't call run_jneuroml + if args.validate_sbml: + from pyneuroml.sbml import validate_sbml_files + validate_sbml_files(" ".join(args.input_files)) + return True + # These do not use the shared option where files are supplied # They require the file name to be specified after # TODO: handle these better diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py new file mode 100644 index 000000000..152dafd3f --- /dev/null +++ b/pyneuroml/sbml/__init__.py @@ -0,0 +1,64 @@ +""" +use libsbml.SBMLDocument.checkConsistency to check validaity of an SBML document +based on https://github.com/combine-org/combine-notebooks/blob/main/src/combine_notebooks/validation/validation_sbml.py +""" + +import os +import libsbml +from libsbml import SBMLDocument, SBMLReader + +def validate_sbml_files(input_files : str,units_consistency: bool = False): + ''' + validate each input file using libsbml.SBMLDocument.checkConsistency + ''' + file_list = input_files.split() + + for file_name in file_list: + if not os.path.isfile(file_name): + raise OSError( + ("Could not find SBML file %s" % file_name) + ) + + try: + reader = SBMLReader() + doc = reader.readSBML(file_name) + except: + raise OSError( + ("SBMLReader failed to load the file %s" % file_name) + ) + + # set the unit checking, similar for the other settings + doc.setConsistencyChecks(libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, units_consistency) + doc.checkConsistency() + # get errors/warnings + n_errors: int = doc.getNumErrors() + errors: List[libsbml.SBMLError] = list() + warnings: List[libsbml.SBMLError] = list() + for k in range(n_errors): + error: libsbml.SBMLError = doc.getError(k) + severity = error.getSeverity() + if (severity == libsbml.LIBSBML_SEV_ERROR) or ( + severity == libsbml.LIBSBML_SEV_FATAL + ): + errors.append(error) + else: + warnings.append(error) + # print results + print("-" * 80) + print(f"{'validation error(s)':<25}: {len(errors)}") + print(f"{'validation warning(s)':<25}: {len(warnings)}") + if len(errors) > 0: + print("--- errors ---") + for kerr in enumerate(errors): + print(f"E{kerr}: {error.getCategoryAsString()} L{error.getLine()}") + print( + f"[{error.getSeverityAsString()}] {error.getShortMessage()} | {error.getMessage()}" + ) + if len(warnings) > 0: + print("--- warnings ---") + for kwarn in enumerate(warnings): + print(f"E{kwarn}: {error.getCategoryAsString()} L{error.getLine()}") + print( + f"[{error.getSeverityAsString()}] {error.getShortMessage()} | {error.getMessage()}" + ) + print("-" * 80) \ No newline at end of file diff --git a/tests/sbml/test_data/test_doc.sbml b/tests/sbml/test_data/test_doc.sbml new file mode 100644 index 000000000..cd05db813 --- /dev/null +++ b/tests/sbml/test_data/test_doc.sbml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + k + S1 + c1 + + + + + + + \ No newline at end of file diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py new file mode 100755 index 000000000..51609f624 --- /dev/null +++ b/tests/sbml/test_sbml.py @@ -0,0 +1,6 @@ +#!/usr/bin/env python3 + +from pyneuroml import sbml + +fname = "test_data/test_doc.sbml" +doc = sbml.validate_sbml_files(fname) From 0b471e1d1769abc7fd38cae5166cb754906dec32 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Thu, 26 Oct 2023 20:14:25 +0000 Subject: [PATCH 02/14] minor tweaks --- pyneuroml/sbml/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 152dafd3f..45ef2bbd7 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -10,10 +10,10 @@ def validate_sbml_files(input_files : str,units_consistency: bool = False): ''' validate each input file using libsbml.SBMLDocument.checkConsistency + input_files is a space separated list of one or more filepaths ''' - file_list = input_files.split() - for file_name in file_list: + for file_name in input_files.split(): if not os.path.isfile(file_name): raise OSError( ("Could not find SBML file %s" % file_name) From ec6f9c425f7199341299836bb5ced3ec631eda56 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Fri, 27 Oct 2023 16:50:53 +0000 Subject: [PATCH 03/14] added libsbml as an optional extra requirement --- setup.cfg | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup.cfg b/setup.cfg index 6c9339eff..07b448104 100644 --- a/setup.cfg +++ b/setup.cfg @@ -101,6 +101,9 @@ plotly = nsg = pynsgr +sbml = + libsbml + all = pyNeuroML[neuron] pyNeuroML[brian] @@ -113,6 +116,7 @@ all = pyNeuroML[vispy] pyNeuroML[plotly] pyNeuroML[nsg] + pyNeuroML[sbml] dev = pyNeuroML[all] From 50d45e37e67f2b43f046e84fce64bfbeee121d90 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Fri, 27 Oct 2023 16:58:43 +0000 Subject: [PATCH 04/14] fix package name --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 07b448104..4e672195b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -102,7 +102,7 @@ nsg = pynsgr sbml = - libsbml + python-libsbml all = pyNeuroML[neuron] From 375e4eee053e9bb365557afc13b05f8647a61bf0 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Mon, 30 Oct 2023 10:26:20 +0000 Subject: [PATCH 05/14] pyneuroml/sbml/__init__.py now passing flake8 and black --- pyneuroml/sbml/__init__.py | 26 +++++++++++++------------- tests/sbml/test_sbml.py | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 45ef2bbd7..615bbfa03 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -5,30 +5,30 @@ import os import libsbml -from libsbml import SBMLDocument, SBMLReader +from libsbml import SBMLReader +from typing import List -def validate_sbml_files(input_files : str,units_consistency: bool = False): - ''' + +def validate_sbml_files(input_files: str, units_consistency: bool = False): + """ validate each input file using libsbml.SBMLDocument.checkConsistency input_files is a space separated list of one or more filepaths - ''' + """ for file_name in input_files.split(): if not os.path.isfile(file_name): - raise OSError( - ("Could not find SBML file %s" % file_name) - ) + raise OSError(("Could not find SBML file %s" % file_name)) try: reader = SBMLReader() doc = reader.readSBML(file_name) - except: - raise OSError( - ("SBMLReader failed to load the file %s" % file_name) - ) + except Exception: + raise OSError(("SBMLReader failed to load the file %s" % file_name)) # set the unit checking, similar for the other settings - doc.setConsistencyChecks(libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, units_consistency) + doc.setConsistencyChecks( + libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, units_consistency + ) doc.checkConsistency() # get errors/warnings n_errors: int = doc.getNumErrors() @@ -61,4 +61,4 @@ def validate_sbml_files(input_files : str,units_consistency: bool = False): print( f"[{error.getSeverityAsString()}] {error.getShortMessage()} | {error.getMessage()}" ) - print("-" * 80) \ No newline at end of file + print("-" * 80) diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index 51609f624..297e9c530 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -2,5 +2,5 @@ from pyneuroml import sbml -fname = "test_data/test_doc.sbml" +fname = "tests/sbml/test_data/test_doc.sbml" doc = sbml.validate_sbml_files(fname) From 883b2cac6ce628fc463791b952787984488bfa9f Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Mon, 30 Oct 2023 10:29:52 +0000 Subject: [PATCH 06/14] pyneuroml/pynml.py changed by black --- pyneuroml/pynml.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index da1c7f5ea..08e60c61f 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -2119,7 +2119,9 @@ def evaluate_arguments(args): # Deal with the SBML validation option which doesn't call run_jneuroml if args.validate_sbml: from pyneuroml.sbml import validate_sbml_files + validate_sbml_files(" ".join(args.input_files)) + return True # These do not use the shared option where files are supplied From 42a391293ebdfd7ab1e60f8797c14b2a15f5fe0a Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Wed, 8 Nov 2023 11:21:35 +0000 Subject: [PATCH 07/14] converted my simple test into a pytest-able function, callable using pytest -k --- tests/sbml/test_sbml.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index 297e9c530..18afdc6be 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -2,5 +2,11 @@ from pyneuroml import sbml -fname = "tests/sbml/test_data/test_doc.sbml" -doc = sbml.validate_sbml_files(fname) + +def test_validate_sbml_files(): + fname = "tests/sbml/test_data/test_doc.sbml" + doc = sbml.validate_sbml_files(fname) + + +if __name__ == "__main__": + test_validate_sbml_files() From 1a078b9aefe33eef8a3adea6523d48142a9c0fa8 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Thu, 9 Nov 2023 12:51:24 +0000 Subject: [PATCH 08/14] improving error handling --- pyneuroml/pynml.py | 36 ++++++++++++++++++++++++++++++------ pyneuroml/sbml/__init__.py | 32 +++++++++++++++++++++++--------- tests/sbml/test_sbml.py | 7 ++++++- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index 08e60c61f..b8ff25516 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -126,8 +126,8 @@ def parse_arguments(): "input_files", type=str, nargs="*", - metavar="", - help="LEMS/NeuroML 2 file(s) to process", + metavar="", + help="LEMS/NeuroML 2/SBML file(s) to process", ) mut_exc_opts_grp = parser.add_argument_group( @@ -348,7 +348,12 @@ def parse_arguments(): mut_exc_opts.add_argument( "-validate-sbml", action="store_true", - help=("Validate SBML file(s)"), + help=("Validate SBML file(s), unit consistency failure generates a warning"), + ) + mut_exc_opts.add_argument( + "-validate-sbml-units", + action="store_true", + help=("Validate SBML file(s), unit consistency failure generates an error"), ) return parser.parse_args() @@ -2117,10 +2122,29 @@ def evaluate_arguments(args): exit_on_fail = True # Deal with the SBML validation option which doesn't call run_jneuroml - if args.validate_sbml: - from pyneuroml.sbml import validate_sbml_files + if args.validate_sbml or args.validate_sbml_units: + try: + from pyneuroml.sbml import validate_sbml_files + except Exception: + logger.critical("Unable to import pyneuroml.sbml") + sys.exit(UNKNOWN_ERR) + + if not len(args.input_files) >= 1: + logger.critical("No input files specified") + sys.exit(ARGUMENT_ERR) + + if args.validate_sbml_units: + strict_units = True + else: + strict_units = False + + try: + result = validate_sbml_files(args.input_files, strict_units) + except Exception as e: + logger.critical(f"validate_sbml_files failed with {e.message}") + sys.exit(UNKNOWN_ERR) - validate_sbml_files(" ".join(args.input_files)) + sys.exit(not result) return True diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 615bbfa03..3a21b92a4 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -9,26 +9,30 @@ from typing import List -def validate_sbml_files(input_files: str, units_consistency: bool = False): +def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> bool: """ validate each input file using libsbml.SBMLDocument.checkConsistency - input_files is a space separated list of one or more filepaths + input_files is a list of one or more filepaths + strict_units converts unit consistency warnings into errors """ - for file_name in input_files.split(): + if not len(input_files) >= 1: + raise Exception("No input files specified") + + no_errors = True + + for file_name in input_files: if not os.path.isfile(file_name): - raise OSError(("Could not find SBML file %s" % file_name)) + raise OSError(f"Could not find SBML file {file_name}") try: reader = SBMLReader() doc = reader.readSBML(file_name) except Exception: - raise OSError(("SBMLReader failed to load the file %s" % file_name)) + raise OSError(f"SBMLReader failed to load the file {file_name}") - # set the unit checking, similar for the other settings - doc.setConsistencyChecks( - libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, units_consistency - ) + # always check for unit consistency + doc.setConsistencyChecks(libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, True) doc.checkConsistency() # get errors/warnings n_errors: int = doc.getNumErrors() @@ -41,6 +45,13 @@ def validate_sbml_files(input_files: str, units_consistency: bool = False): severity == libsbml.LIBSBML_SEV_FATAL ): errors.append(error) + elif ( + (strict_units == True) + and (error.getErrorId() >= 10600) + and (error.getErrorId() <= 10700) + ): + # treat unit consistency warnings as errors + errors.append(error) else: warnings.append(error) # print results @@ -48,6 +59,7 @@ def validate_sbml_files(input_files: str, units_consistency: bool = False): print(f"{'validation error(s)':<25}: {len(errors)}") print(f"{'validation warning(s)':<25}: {len(warnings)}") if len(errors) > 0: + no_errors = False print("--- errors ---") for kerr in enumerate(errors): print(f"E{kerr}: {error.getCategoryAsString()} L{error.getLine()}") @@ -62,3 +74,5 @@ def validate_sbml_files(input_files: str, units_consistency: bool = False): f"[{error.getSeverityAsString()}] {error.getShortMessage()} | {error.getMessage()}" ) print("-" * 80) + + return no_errors diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index 18afdc6be..119f95018 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -5,8 +5,13 @@ def test_validate_sbml_files(): fname = "tests/sbml/test_data/test_doc.sbml" - doc = sbml.validate_sbml_files(fname) + doc = sbml.validate_sbml_files([fname]) + + +def test_validate_sbml_files2(): + doc = sbml.validate_sbml_files([]) if __name__ == "__main__": test_validate_sbml_files() + test_validate_sbml_files2() From 056e76298aaf8d801b2b243f74f58e54f8f3d118 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Fri, 10 Nov 2023 16:00:24 +0000 Subject: [PATCH 09/14] tidied up error handling, corrected error code range --- pyneuroml/pynml.py | 10 ++++++++-- pyneuroml/sbml/__init__.py | 30 ++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index b8ff25516..edd98749b 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -2134,8 +2134,10 @@ def evaluate_arguments(args): sys.exit(ARGUMENT_ERR) if args.validate_sbml_units: + # A failed unit consistency check generates an error strict_units = True else: + # A failed unit consistency check generates only a warning strict_units = False try: @@ -2144,9 +2146,13 @@ def evaluate_arguments(args): logger.critical(f"validate_sbml_files failed with {e.message}") sys.exit(UNKNOWN_ERR) - sys.exit(not result) + if result: + # All files validated ok (with possible warnings but no errors) + sys.exit(0) - return True + # Errors of some kind were found in one or more files + logger.error(f"one or more SBML files failed to validate") + sys.exit(UNKNOWN_ERR) # These do not use the shared option where files are supplied # They require the file name to be specified after diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 3a21b92a4..9edde5a06 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -19,25 +19,28 @@ def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> b if not len(input_files) >= 1: raise Exception("No input files specified") - no_errors = True + all_valid = True for file_name in input_files: if not os.path.isfile(file_name): raise OSError(f"Could not find SBML file {file_name}") + reader = SBMLReader() + try: - reader = SBMLReader() doc = reader.readSBML(file_name) except Exception: raise OSError(f"SBMLReader failed to load the file {file_name}") - # always check for unit consistency + # Always check for unit consistency doc.setConsistencyChecks(libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, True) doc.checkConsistency() - # get errors/warnings + + # Get errors/warnings n_errors: int = doc.getNumErrors() errors: List[libsbml.SBMLError] = list() warnings: List[libsbml.SBMLError] = list() + for k in range(n_errors): error: libsbml.SBMLError = doc.getError(k) severity = error.getSeverity() @@ -46,26 +49,32 @@ def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> b ): errors.append(error) elif ( - (strict_units == True) - and (error.getErrorId() >= 10600) - and (error.getErrorId() <= 10700) + (strict_units is True) + # For error code definitions see + # https://github.com/sbmlteam/libsbml/blob/fee56c943ea39b9ac1f8491cac2fc9b3665e368f/src/sbml/SBMLError.h#L528 + # and sbml.level-3.version-2.core.release-2.pdf page 159 + and (error.getErrorId() >= 10500) + and (error.getErrorId() <= 10599) ): - # treat unit consistency warnings as errors + # Treat unit consistency warnings as errors errors.append(error) else: warnings.append(error) + # print results print("-" * 80) print(f"{'validation error(s)':<25}: {len(errors)}") print(f"{'validation warning(s)':<25}: {len(warnings)}") + if len(errors) > 0: - no_errors = False + all_valid = False print("--- errors ---") for kerr in enumerate(errors): print(f"E{kerr}: {error.getCategoryAsString()} L{error.getLine()}") print( f"[{error.getSeverityAsString()}] {error.getShortMessage()} | {error.getMessage()}" ) + if len(warnings) > 0: print("--- warnings ---") for kwarn in enumerate(warnings): @@ -73,6 +82,7 @@ def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> b print( f"[{error.getSeverityAsString()}] {error.getShortMessage()} | {error.getMessage()}" ) + print("-" * 80) - return no_errors + return all_valid From beb73ed9c77a8e9045718a9699aaffd6439940ae Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Fri, 10 Nov 2023 17:59:32 +0000 Subject: [PATCH 10/14] expanded the sbml test function --- tests/sbml/test_data/invalid_doc00.sbml | 2 + tests/sbml/test_data/invalid_doc01.sbml | 42 ++++++++++++++++++ tests/sbml/test_data/invalid_doc02.sbml | 42 ++++++++++++++++++ .../{test_doc.sbml => valid_doc.sbml} | 2 +- tests/sbml/test_sbml.py | 43 ++++++++++++++++--- 5 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 tests/sbml/test_data/invalid_doc00.sbml create mode 100644 tests/sbml/test_data/invalid_doc01.sbml create mode 100644 tests/sbml/test_data/invalid_doc02.sbml rename tests/sbml/test_data/{test_doc.sbml => valid_doc.sbml} (99%) diff --git a/tests/sbml/test_data/invalid_doc00.sbml b/tests/sbml/test_data/invalid_doc00.sbml new file mode 100644 index 000000000..923ce7010 --- /dev/null +++ b/tests/sbml/test_data/invalid_doc00.sbml @@ -0,0 +1,2 @@ +10 PRINT "HELLO" +20 GOTO 10 diff --git a/tests/sbml/test_data/invalid_doc01.sbml b/tests/sbml/test_data/invalid_doc01.sbml new file mode 100644 index 000000000..011c3c9b2 --- /dev/null +++ b/tests/sbml/test_data/invalid_doc01.sbml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + k + S1 + c1 + + + + + + + diff --git a/tests/sbml/test_data/invalid_doc02.sbml b/tests/sbml/test_data/invalid_doc02.sbml new file mode 100644 index 000000000..05d999017 --- /dev/null +++ b/tests/sbml/test_data/invalid_doc02.sbml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + k + S1 + c1 + + + + + + + diff --git a/tests/sbml/test_data/test_doc.sbml b/tests/sbml/test_data/valid_doc.sbml similarity index 99% rename from tests/sbml/test_data/test_doc.sbml rename to tests/sbml/test_data/valid_doc.sbml index cd05db813..578594a3b 100644 --- a/tests/sbml/test_data/test_doc.sbml +++ b/tests/sbml/test_data/valid_doc.sbml @@ -39,4 +39,4 @@ - \ No newline at end of file + diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index 119f95018..59958c548 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -3,15 +3,48 @@ from pyneuroml import sbml -def test_validate_sbml_files(): - fname = "tests/sbml/test_data/test_doc.sbml" - doc = sbml.validate_sbml_files([fname]) +def test_validate_sbml_files1(): + "ensure it validates a single valid file by returning True" + + fname = "tests/sbml/test_data/valid_doc.sbml" + result = sbml.validate_sbml_files([fname]) + assert result def test_validate_sbml_files2(): - doc = sbml.validate_sbml_files([]) + "ensure it raises an exception for failing to provide any files" + + try: + result = sbml.validate_sbml_files([]) + except Exception: + return + + raise Exception("failed to flag missing input files") + + +def test_validate_sbml_files3(): + """ + ensure it returns False for all invalid files + without raising any exception + """ + + fail_count = 0 + n_files = 3 + + for i in range(n_files): + fname = "tests/sbml/test_data/invalid_doc%02d.sbml" % i + + try: + result = sbml.validate_sbml_files([fname]) + if not result: + fail_count += 1 + except Exception: + pass + + assert fail_count == n_files if __name__ == "__main__": - test_validate_sbml_files() + test_validate_sbml_files1() test_validate_sbml_files2() + test_validate_sbml_files3() From 8b2ea3f5133c99b95a0543e0fa31d1a1277fdfa5 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Mon, 13 Nov 2023 13:02:20 +0000 Subject: [PATCH 11/14] more accurate exception handling --- pyneuroml/sbml/__init__.py | 9 ++++++--- tests/sbml/test_sbml.py | 10 ++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 9edde5a06..3a984bdad 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -17,20 +17,23 @@ def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> b """ if not len(input_files) >= 1: - raise Exception("No input files specified") + raise ValueError("No input files specified") all_valid = True for file_name in input_files: if not os.path.isfile(file_name): - raise OSError(f"Could not find SBML file {file_name}") + raise FileNotFoundError(f"Could not find SBML file {file_name}") + + if not os.access(file_name, os.R_OK): + raise IOError(f"Could not read SBML file {file_name}") reader = SBMLReader() try: doc = reader.readSBML(file_name) except Exception: - raise OSError(f"SBMLReader failed to load the file {file_name}") + raise IOError(f"SBMLReader failed to load the file {file_name}") # Always check for unit consistency doc.setConsistencyChecks(libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, True) diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index 59958c548..d7dbd9998 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -12,20 +12,22 @@ def test_validate_sbml_files1(): def test_validate_sbml_files2(): - "ensure it raises an exception for failing to provide any files" + "ensure it raises a ValueError exception for failing to provide any files" try: result = sbml.validate_sbml_files([]) - except Exception: + except ValueError: return + except Exception: + pass - raise Exception("failed to flag missing input files") + raise Exception("failed to properly flag missing input files") def test_validate_sbml_files3(): """ ensure it returns False for all invalid files - without raising any exception + without raising any exceptions """ fail_count = 0 From a401d8303a9096ecbdf105a0a002b74a6e2ac7cf Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Mon, 13 Nov 2023 13:31:00 +0000 Subject: [PATCH 12/14] added a test for both unit inconsistency check modes --- .../test_data/inconsistent_units_doc.sbml | 42 +++++++++++++++++++ tests/sbml/test_sbml.py | 36 +++++++++++++--- 2 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 tests/sbml/test_data/inconsistent_units_doc.sbml diff --git a/tests/sbml/test_data/inconsistent_units_doc.sbml b/tests/sbml/test_data/inconsistent_units_doc.sbml new file mode 100644 index 000000000..998a448b4 --- /dev/null +++ b/tests/sbml/test_data/inconsistent_units_doc.sbml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + k + S1 + S2 + + + + + + + diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index d7dbd9998..8c29432fe 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -3,7 +3,7 @@ from pyneuroml import sbml -def test_validate_sbml_files1(): +def test_sbml_validate_a_valid_file(): "ensure it validates a single valid file by returning True" fname = "tests/sbml/test_data/valid_doc.sbml" @@ -11,7 +11,7 @@ def test_validate_sbml_files1(): assert result -def test_validate_sbml_files2(): +def test_sbml_validate_valueerror_on_no_inputfiles(): "ensure it raises a ValueError exception for failing to provide any files" try: @@ -24,7 +24,30 @@ def test_validate_sbml_files2(): raise Exception("failed to properly flag missing input files") -def test_validate_sbml_files3(): +def test_sbml_validate_unit_consistency_check(): + """ + ensure it fails a unit inconsistency in strict mode + ensure it only warns about a unit inconsistency when not in strict mode + """ + + try: + result = sbml.validate_sbml_files( + ["tests/sbml/test_data/inconsistent_units_doc.sbml"], strict_units=True + ) + assert not result + except Exception: + raise Exception("failed to flag inconsistent units as an error") + + try: + result = sbml.validate_sbml_files( + ["tests/sbml/test_data/inconsistent_units_doc.sbml"], strict_units=False + ) + assert result + except Exception: + raise Exception("failed to flag inconsistent units as an error") + + +def test_sbml_validate_flag_all_invalid_files(): """ ensure it returns False for all invalid files without raising any exceptions @@ -47,6 +70,7 @@ def test_validate_sbml_files3(): if __name__ == "__main__": - test_validate_sbml_files1() - test_validate_sbml_files2() - test_validate_sbml_files3() + test_sbml_validate_validate_a_valid_file() + test_sbml_validate_valueerror_on_no_inputfiles() + test_sbml_validate_flag_all_invalid_files() + test_sbml_validate_unit_consistency_check() From d30e57dfe4795d5b39832f10b443b5c317c3c16c Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Mon, 13 Nov 2023 15:23:00 +0000 Subject: [PATCH 13/14] added two more test cases --- pyneuroml/sbml/__init__.py | 10 +++--- tests/sbml/test_data/no_read_access.sbml | 42 ++++++++++++++++++++++++ tests/sbml/test_sbml.py | 34 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 tests/sbml/test_data/no_read_access.sbml diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 3a984bdad..6a4e2c5fe 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -22,24 +22,26 @@ def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> b all_valid = True for file_name in input_files: + # These checks are already implemented by SBMLReader + # But could just be logged along with the other error types rather than causing exceptions if not os.path.isfile(file_name): raise FileNotFoundError(f"Could not find SBML file {file_name}") if not os.access(file_name, os.R_OK): raise IOError(f"Could not read SBML file {file_name}") - reader = SBMLReader() - try: + reader = SBMLReader() doc = reader.readSBML(file_name) except Exception: - raise IOError(f"SBMLReader failed to load the file {file_name}") + # usually errors are logged within the doc object rather than being thrown + raise IOError(f"SBMLReader failed trying to open the file {file_name}") # Always check for unit consistency doc.setConsistencyChecks(libsbml.LIBSBML_CAT_UNITS_CONSISTENCY, True) doc.checkConsistency() - # Get errors/warnings + # Get errors/warnings arising from the file reading or consistency checking calls above n_errors: int = doc.getNumErrors() errors: List[libsbml.SBMLError] = list() warnings: List[libsbml.SBMLError] = list() diff --git a/tests/sbml/test_data/no_read_access.sbml b/tests/sbml/test_data/no_read_access.sbml new file mode 100644 index 000000000..011c3c9b2 --- /dev/null +++ b/tests/sbml/test_data/no_read_access.sbml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + k + S1 + c1 + + + + + + + diff --git a/tests/sbml/test_sbml.py b/tests/sbml/test_sbml.py index 8c29432fe..8b5afd4b6 100755 --- a/tests/sbml/test_sbml.py +++ b/tests/sbml/test_sbml.py @@ -1,6 +1,8 @@ #!/usr/bin/env python3 from pyneuroml import sbml +import os +import stat def test_sbml_validate_a_valid_file(): @@ -11,6 +13,36 @@ def test_sbml_validate_a_valid_file(): assert result +def test_sbml_validate_missing_inputfile(): + try: + result = sbml.validate_sbml_files(["tests/sbml/test_data/nonexistent_file"]) + except FileNotFoundError: + return + except Exception: + pass + + raise Exception("failed to properly flag file not found error") + + +def test_sbml_validate_no_read_access(): + fname = "tests/sbml/test_data/no_read_access.sbml" + + # Remove read permission + os.chmod(fname, 0) + try: + result = sbml.validate_sbml_files([fname]) + except IOError: + os.chmod(fname, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) + return + except Exception: + pass + + os.chmod( + fname, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH + ) # Restore permission for git + raise Exception("failed to properly flag lack of read access") + + def test_sbml_validate_valueerror_on_no_inputfiles(): "ensure it raises a ValueError exception for failing to provide any files" @@ -72,5 +104,7 @@ def test_sbml_validate_flag_all_invalid_files(): if __name__ == "__main__": test_sbml_validate_validate_a_valid_file() test_sbml_validate_valueerror_on_no_inputfiles() + test_sbml_validate_missing_inputfile() test_sbml_validate_flag_all_invalid_files() test_sbml_validate_unit_consistency_check() + test_sbml_validate_no_read_access() From c358b2f83b6e3432f3d5486db0ae0037ebae8ef8 Mon Sep 17 00:00:00 2001 From: Robert Vickerstaff Date: Mon, 13 Nov 2023 15:50:59 +0000 Subject: [PATCH 14/14] added simple examples to test-ghactions.sh --- examples/test_data/valid_doc.sbml | 42 +++++++++++++++++++++++++++++++ pyneuroml/pynml.py | 2 +- pyneuroml/sbml/__init__.py | 3 ++- test-ghactions.sh | 7 ++++++ 4 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 examples/test_data/valid_doc.sbml diff --git a/examples/test_data/valid_doc.sbml b/examples/test_data/valid_doc.sbml new file mode 100644 index 000000000..578594a3b --- /dev/null +++ b/examples/test_data/valid_doc.sbml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + k + S1 + c1 + + + + + + + diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py index edd98749b..2efc4f320 100644 --- a/pyneuroml/pynml.py +++ b/pyneuroml/pynml.py @@ -2143,7 +2143,7 @@ def evaluate_arguments(args): try: result = validate_sbml_files(args.input_files, strict_units) except Exception as e: - logger.critical(f"validate_sbml_files failed with {e.message}") + logger.critical(f"validate_sbml_files failed with {str(e)}") sys.exit(UNKNOWN_ERR) if result: diff --git a/pyneuroml/sbml/__init__.py b/pyneuroml/sbml/__init__.py index 6a4e2c5fe..c1ec3cf00 100644 --- a/pyneuroml/sbml/__init__.py +++ b/pyneuroml/sbml/__init__.py @@ -4,6 +4,7 @@ """ import os +import errno import libsbml from libsbml import SBMLReader from typing import List @@ -25,7 +26,7 @@ def validate_sbml_files(input_files: List[str], strict_units: bool = False) -> b # These checks are already implemented by SBMLReader # But could just be logged along with the other error types rather than causing exceptions if not os.path.isfile(file_name): - raise FileNotFoundError(f"Could not find SBML file {file_name}") + raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), file_name) if not os.access(file_name, os.R_OK): raise IOError(f"Could not read SBML file {file_name}") diff --git a/test-ghactions.sh b/test-ghactions.sh index aa261a1d3..4fd7c1e84 100755 --- a/test-ghactions.sh +++ b/test-ghactions.sh @@ -70,6 +70,13 @@ pynml LEMS_NML2_Ex9_FN.xml -spineml pynml LEMS_NML2_Ex9_FN.xml -sbml +echo +echo "################################################" +echo "## Simple SBML validation example" + +pynml -validate-sbml test_data/valid_doc.sbml +pynml -validate-sbml-units test_data/valid_doc.sbml + echo echo "################################################"