Merge pull request #584 from opencobra/test_sbml_version

Test sbml version

HISTORY.rst

@@ -47,6 +47,12 @@ Next Release
 * Refactor test that identifies duplicate metabolites to use for inchi
   strings in addition to inchikeys.
 * Round score to and display a single decimal value.
+* Read SBML files with modified parser that can collect the level, version and
+  whether the FBC package is used.
+* Validate the SBML structure with the libSBML python API if the parser errors
+  and produce a simple SBML validation report.
+* Add test cases that report on the level and version, and FBC availability
+  through the memote reports.
 
 0.8.11 (2019-01-07)
 -------------------

memote/suite/api.py

@@ -20,20 +20,51 @@
 from __future__ import absolute_import
 
 import logging
+import os
+from io import open
 
 import pytest
+from jinja2 import Environment, PackageLoader, select_autoescape
 
 from memote.suite import TEST_DIRECTORY
 from memote.suite.collect import ResultCollectionPlugin
 from memote.suite.reporting import (
     SnapshotReport, DiffReport, HistoryReport, ReportConfiguration)
+from memote.support import validation as val
 
-__all__ = ("test_model", "snapshot_report", "diff_report", "history_report")
+__all__ = ("validate_model", "test_model", "snapshot_report", "diff_report",
+           "history_report")
 
 LOGGER = logging.getLogger(__name__)
 
 
-def test_model(model, results=False, pytest_args=None,
+def validate_model(path):
+    """
+    Validate a model structurally and optionally store results as JSON.
+
+    Parameters
+    ----------
+    path :
+        Path to model file.
+
+    Returns
+    -------
+    tuple
+        cobra.Model
+            The metabolic model under investigation.
+        tuple
+            A tuple reporting on the SBML level, version, and FBC package
+            version used (if any) in the SBML document.
+        dict
+            A simple dictionary containing a list of errors and warnings.
+
+    """
+    notifications = {"warnings": [], "errors": []}
+    model, sbml_ver = val.load_cobra_model(path, notifications)
+    return model, sbml_ver, notifications
+
+
+def test_model(model, sbml_version=None, results=False, pytest_args=None,
                exclusive=None, skip=None, experimental=None):
     """
     Test a model and optionally store results as JSON.
@@ -42,6 +73,8 @@ def test_model(model, results=False, pytest_args=None,
     ----------
     model : cobra.Model
         The metabolic model under investigation.
+    sbml_version: tuple, optional
+        A tuple reporting on the level, version, and FBC use of the SBML file.
     results : bool, optional
         Whether to return the results in addition to the return code.
     pytest_args : list, optional
@@ -66,7 +99,8 @@ def test_model(model, results=False, pytest_args=None,
         pytest_args.extend(["--tb", "short"])
     if TEST_DIRECTORY not in pytest_args:
         pytest_args.append(TEST_DIRECTORY)
-    plugin = ResultCollectionPlugin(model, exclusive=exclusive, skip=skip,
+    plugin = ResultCollectionPlugin(model, sbml_version=sbml_version,
+                                    exclusive=exclusive, skip=skip,
                                     experimental_config=experimental)
     code = pytest.main(pytest_args, plugins=[plugin])
     if results:
@@ -127,7 +161,7 @@ def diff_report(diff_results, config=None, html=True):
 
     Parameters
     ----------
-    result : memote.MemoteResult
+    diff_results : iterable of memote.MemoteResult
         Nested dictionary structure as returned from the test suite.
     config : dict, optional
         The final test report configuration (default None).
@@ -142,3 +176,25 @@ def diff_report(diff_results, config=None, html=True):
         return report.render_html()
     else:
         return report.render_json()
+
+
+def validation_report(path, notifications, filename):
+    """
+    Generate a validation report from a notification object.
+
+    Parameters
+    ----------
+    path : string
+        Path to model file.
+    notifications : dict
+        A simple dictionary structure containing a list of errors and warnings.
+
+    """
+    env = Environment(
+        loader=PackageLoader('memote.suite', 'templates'),
+        autoescape=select_autoescape(['html', 'xml'])
+    )
+    template = env.get_template('validation_template.html')
+    model = os.path.basename(path)
+    with open(filename, "w") as file_h:
+        file_h.write(template.render(model=model, notifications=notifications))

memote/suite/cli/callbacks.py

@@ -22,41 +22,14 @@
 import shlex
 import sys
 import logging
-import warnings
 
 import click
 import git
-from cobra.io import read_sbml_model
 
 from memote.experimental import ExperimentConfiguration
 
-LOGGER = logging.getLogger(__name__)
-
-
-def _load_model(filename):
-    """
-    Load the model defined in SBMLFBCv2.
-
-    Loading the model uses Cobrapy which has native support for reading and
-    writing SBML with FBCv2.
-
-    Parameters
-    ----------
-    filename: click.Path
 
-    """
-    # TODO: Record the SBML warnings and add them to the report.
-    with warnings.catch_warnings():
-        warnings.simplefilter("ignore", UserWarning)
-        return read_sbml_model(filename)
-
-
-def validate_model(context, param, value):
-    """Load model from path if it exists."""
-    if value is not None:
-        return _load_model(value)
-    else:
-        raise click.BadParameter("No 'model' path given or configured.")
+LOGGER = logging.getLogger(__name__)
 
 
 def validate_collect(context, param, value):

memote/suite/cli/reports.py

@@ -32,6 +32,7 @@
 import os
 
 import memote.suite.api as api
+import memote.utils as utils
 import memote.suite.results as managers
 import memote.suite.cli.callbacks as callbacks
 from memote.suite.cli import CONTEXT_SETTINGS
@@ -50,8 +51,7 @@ def report():
 @report.command(context_settings=CONTEXT_SETTINGS)
 @click.help_option("--help", "-h")
 @click.argument("model", type=click.Path(exists=True, dir_okay=False),
-                envvar="MEMOTE_MODEL",
-                callback=callbacks.validate_model)
+                envvar="MEMOTE_MODEL")
 @click.option("--filename", type=click.Path(exists=False, writable=True),
               default="index.html", show_default=True,
               help="Path for the HTML report output.")
@@ -94,6 +94,14 @@ def snapshot(model, filename, pytest_args, exclusive, skip, solver,
     MODEL: Path to model file. Can also be supplied via the environment variable
     MEMOTE_MODEL or configured in 'setup.cfg' or 'memote.ini'.
     """
+    model_obj, sbml_ver, notifications = api.validate_model(
+        model)
+    if model_obj is None:
+        LOGGER.critical(
+            "The model could not be loaded due to the following SBML errors.")
+        utils.stdout_notifications(notifications)
+        api.validation_report(model, notifications, filename)
+        sys.exit(1)
     if not any(a.startswith("--tb") for a in pytest_args):
         pytest_args = ["--tb", "no"] + pytest_args
     # Add further directories to search for tests.
@@ -102,10 +110,10 @@ def snapshot(model, filename, pytest_args, exclusive, skip, solver,
     # Update the default test configuration with custom ones (if any).
     for custom in custom_config:
         config.merge(ReportConfiguration.load(custom))
-    model.solver = solver
-    _, results = api.test_model(model, results=True, pytest_args=pytest_args,
-                                skip=skip, exclusive=exclusive,
-                                experimental=experimental)
+    model_obj.solver = solver
+    _, results = api.test_model(model_obj, sbml_version=sbml_ver, results=True,
+                                pytest_args=pytest_args, skip=skip,
+                                exclusive=exclusive, experimental=experimental)
     with open(filename, "w", encoding="utf-8") as file_handle:
         LOGGER.info("Writing snapshot report to '%s'.", filename)
         file_handle.write(api.snapshot_report(results, config))
@@ -135,8 +143,6 @@ def snapshot(model, filename, pytest_args, exclusive, skip, solver,
                    "option can be specified multiple times.")
 def history(location, model, filename, deployment, custom_config):
     """Generate a report over a model's git commit history."""
-    if model is None:
-        raise click.BadParameter("No 'model' path given or configured.")
     if location is None:
         raise click.BadParameter("No 'location' given or configured.")
     try:
@@ -165,9 +171,11 @@ def history(location, model, filename, deployment, custom_config):
         file_handle.write(report)
 
 
-def _test_diff(model, pytest_args, skip, exclusive, experimental):
+def _test_diff(model_and_model_ver_tuple, pytest_args, skip,
+               exclusive, experimental):
+    model, sbml_ver = model_and_model_ver_tuple
     _, diff_results = api.test_model(
-        model, results=True, pytest_args=pytest_args,
+        model, sbml_version=sbml_ver, results=True, pytest_args=pytest_args,
         skip=skip, exclusive=exclusive, experimental=experimental)
     return diff_results
 
@@ -228,33 +236,43 @@ def diff(models, filename, pytest_args, exclusive, skip, solver,
         config.merge(ReportConfiguration.load(custom))
     # Build the diff report specific data structure
     diff_results = dict()
-    loaded_models = list()
+    model_and_model_ver_tuple = list()
     for model_path in models:
         try:
             model_filename = os.path.basename(model_path)
             diff_results.setdefault(model_filename, dict())
-            model = callbacks._load_model(model_path)
+            model, model_ver, notifications = api.validate_model(model_path)
+            if model is None:
+                head, tail = os.path.split(filename)
+                report_path = os.path.join(
+                    head, '{}_structural_report.html'.format(model_filename))
+                api.validation_report(
+                    model_path, notifications, report_path)
+                LOGGER.critical(
+                    "The model {} could not be loaded due to SBML errors "
+                    "reported in {}.".format(model_filename, report_path))
+                continue
             model.solver = solver
-            loaded_models.append(model)
+            model_and_model_ver_tuple.append((model, model_ver))
         except (IOError, SBMLError):
             LOGGER.debug(exc_info=True)
             LOGGER.warning("An error occurred while loading the model '%s'. "
                            "Skipping.", model_filename)
     # Abort the diff report unless at least two models can be loaded
     # successfully.
-    if len(loaded_models) < 2:
+    if len(model_and_model_ver_tuple) < 2:
         LOGGER.critical(
             "Out of the %d provided models only %d could be loaded. Please, "
             "check if the models that could not be loaded are valid SBML. "
             "Aborting.",
-            len(models), len(loaded_models))
+            len(models), len(model_and_model_ver_tuple))
         sys.exit(1)
     # Running pytest in individual processes to avoid interference
     partial_test_diff = partial(_test_diff, pytest_args=pytest_args,
                                 skip=skip, exclusive=exclusive,
                                 experimental=experimental)
     pool = Pool(min(len(models), os.cpu_count()))
-    results = pool.map(partial_test_diff, loaded_models)
+    results = pool.map(partial_test_diff, model_and_model_ver_tuple)
 
     for model_path, result in zip(models, results):
         model_filename = os.path.basename(model_path)

memote/suite/cli/runner.py

@@ -34,7 +34,6 @@
 import click_log
 import git
 import travis.encrypt as te
-from cobra.io import read_sbml_model
 from cookiecutter.main import cookiecutter, get_user_config
 from github import (
     Github, BadCredentialsException, UnknownObjectException, GithubException)
@@ -50,7 +49,7 @@
 from memote.suite.cli.reports import report
 from memote.suite.results import (
     ResultManager, RepoResultManager, SQLResultManager, HistoryManager)
-from memote.utils import is_modified
+from memote.utils import is_modified, stdout_notifications
 
 
 LOGGER = logging.getLogger()
@@ -126,7 +125,8 @@ def cli():
 @click.argument("model", type=click.Path(exists=True, dir_okay=False),
                 envvar="MEMOTE_MODEL")
 def run(model, collect, filename, location, ignore_git, pytest_args, exclusive,
-        skip, solver, experimental, custom_tests, deployment, skip_unchanged):
+        skip, solver, experimental, custom_tests, deployment,
+        skip_unchanged):
     """
     Run the test suite on a single model and collect results.
 
@@ -164,10 +164,16 @@ def is_verbose(arg):
     # Add further directories to search for tests.
     pytest_args.extend(custom_tests)
     # Check if the model can be loaded at all.
-    model = callbacks.validate_model(None, None, model)
+    model, sbml_ver, notifications = api.validate_model(model)
+    if model is None:
+        LOGGER.critical(
+            "The model could not be loaded due to the following SBML errors.")
+        stdout_notifications(notifications)
+        sys.exit(1)
     model.solver = solver
     code, result = api.test_model(
-        model=model, results=True, pytest_args=pytest_args, skip=skip,
+        model=model, sbml_version=sbml_ver, results=True,
+        pytest_args=pytest_args, skip=skip,
         exclusive=exclusive, experimental=experimental)
     if collect:
         if repo is None:
@@ -234,18 +240,18 @@ def new(directory, replay):
 def _model_from_stream(stream, filename):
     if filename.endswith(".gz"):
         with GzipFile(fileobj=stream) as file_handle:
-            model = read_sbml_model(file_handle)
+            model, sbml_ver, notifications = api.validate_model(file_handle)
     else:
-        model = read_sbml_model(stream)
-    return model
+        model, sbml_ver, notifications = api.validate_model(stream)
+    return model, sbml_ver, notifications
 
 
-def _test_history(model, solver, manager, commit, pytest_args, skip,
+def _test_history(model, sbml_ver, solver, manager, commit, pytest_args, skip,
                   exclusive, experimental):
     model.solver = solver
     _, result = api.test_model(
-        model, results=True, pytest_args=pytest_args, skip=skip,
-        exclusive=exclusive, experimental=experimental)
+        model, sbml_version=sbml_ver, results=True, pytest_args=pytest_args,
+        skip=skip, exclusive=exclusive, experimental=experimental)
     manager.store(result, commit=commit)
 
 
@@ -300,8 +306,7 @@ def history(model, message, rewrite, solver, location, pytest_args, deployment,
        for those only. This can also be achieved by supplying a commit range.
 
     """
-    if model is None:
-        raise click.BadParameter("No 'model' path given or configured.")
+    callbacks.validate_path(model)
     if location is None:
         raise click.BadParameter("No 'location' given or configured.")
     if "--tb" not in pytest_args:
@@ -364,11 +369,18 @@ def history(model, message, rewrite, solver, location, pytest_args, deployment,
         LOGGER.info(
             "Running the test suite for commit '{}'.".format(commit))
         blob = cmt.tree[model]
-        model_obj = _model_from_stream(blob.data_stream, blob.name)
+        model_obj, sbml_ver, notifications = _model_from_stream(
+            blob.data_stream, blob.name
+        )
+        if model_obj is None:
+            LOGGER.critical("The model could not be loaded due to the "
+                            "following SBML errors.")
+            stdout_notifications(notifications)
+            continue
         proc = Process(
             target=_test_history,
-            args=(model_obj, solver, manager, commit, pytest_args, skip,
-                  exclusive, experimental))
+            args=(model_obj, sbml_ver, solver, manager, commit,
+                  pytest_args, skip, exclusive, experimental))
         proc.start()
         proc.join()
     # Copy back all new and modified files and add them to the index.