Merge pull request #622 from opencobra/fix-experiments

Fix experiments

HISTORY.rst

@@ -3,6 +3,8 @@ History
 
 Next Release
 ------------
+* Repair the experimental testing and add a test case for the runner with
+  experimental data.
 * Update docstring of test_find_duplicate_reactions.
 * Add a guide explaining the structure of and how to interpret the memote
   reports.

memote/experimental/config.py

@@ -64,17 +64,16 @@ def __init__(self, filename, **kwargs):
         self.essentiality = dict()
         self.growth = dict()
 
-    def load_data(self, model):
+    def load(self, model):
         """
-        Load a configuration file and validate it.
+        Load all information from an experimental configuration file.
 
         Parameters
         ----------
         model : cobra.Model
             The metabolic model under investigation.
 
         """
-        self.validate()
         self.load_medium(model)
         self.load_essentiality(model)
         self.load_growth(model)

memote/suite/cli/runner.py

@@ -171,6 +171,9 @@ def is_verbose(arg):
         stdout_notifications(notifications)
         sys.exit(1)
     model.solver = solver
+    # Load the experimental configuration using model information.
+    if experimental is not None:
+        experimental.load(model)
     code, result = api.test_model(
         model=model, sbml_version=sbml_ver, results=True,
         pytest_args=pytest_args, skip=skip,
@@ -249,6 +252,9 @@ def _model_from_stream(stream, filename):
 def _test_history(model, sbml_ver, solver, manager, commit, pytest_args, skip,
                   exclusive, experimental):
     model.solver = solver
+    # Load the experimental configuration using model information.
+    if experimental is not None:
+        experimental.load(model)
     _, result = api.test_model(
         model, sbml_version=sbml_ver, results=True, pytest_args=pytest_args,
         skip=skip, exclusive=exclusive, experimental=experimental)

memote/suite/collect.py

@@ -81,19 +81,19 @@ def pytest_generate_tests(self, metafunc):
             metafunc.parametrize("reaction_id", [
                 rxn.id for rxn in find_biomass_reaction(self._model)])
             return
-        if metafunc.definition.get_closest_marker("essentiality"):
-            if self._exp_config is None:
+        # Parametrize experimental test cases.
+        for kind in ["essentiality", "growth"]:
+            # Find a corresponding pytest marker on the test case.
+            if not metafunc.definition.get_closest_marker(kind):
+                continue
+            exp = getattr(self._exp_config, kind, None)
+            if exp is None:
                 metafunc.parametrize("experiment", [])
             else:
-                metafunc.parametrize("experiment",
-                                     self._exp_config.get("essentiality", []))
-            return
-        if metafunc.definition.get_closest_marker("growth"):
-            if self._exp_config is None:
-                metafunc.parametrize("experiment", [])
-            else:
-                metafunc.parametrize("experiment",
-                                     self._exp_config.get("growth", []))
+                metafunc.parametrize(
+                    "experiment", list(exp.items()))
+            # We only expect one kind of experimental marker per test case
+            # and thus end execution here.
             return
 
     @pytest.hookimpl(tryfirst=True)

memote/suite/tests/test_essentiality.py

@@ -55,18 +55,18 @@ def test_gene_essentiality_from_data_qualitative(model, experiment,
 
     """
     ann = test_gene_essentiality_from_data_qualitative.annotation
-    exp = pytest.memote.experimental.essentiality[experiment]
+    name, exp = experiment
     expected = exp.data
     test = exp.evaluate(model)
-    ann["data"][experiment] = confusion_matrix(
+    ann["data"][name] = result = confusion_matrix(
         set(test.loc[test["essential"], "gene"]),
         set(expected.loc[expected["essential"], "gene"]),
         set(test.loc[~test["essential"], "gene"]),
         set(expected.loc[~expected["essential"], "gene"])
     )
-    ann["metric"][experiment] = ann["data"][experiment]["ACC"]
-    ann["message"][experiment] = wrapper.fill(
+    ann["metric"][name] = result["ACC"]
+    ann["message"][name] = wrapper.fill(
         """Ideally, every model would show a perfect accuracy of 1. In
-        experiment '{}' the model has  {:.2}.""".format(
-            experiment, ann["data"][experiment]["MCC"]))
-    assert ann["data"][experiment]["ACC"] > threshold
+        name '{}' the model has  {:.2}.""".format(
+            name, result["MCC"]))
+    assert result["ACC"] > threshold

memote/suite/tests/test_growth.py

@@ -54,20 +54,20 @@ def test_growth_from_data_qualitative(model, experiment, threshold=0.95):
 
     """
     ann = test_growth_from_data_qualitative.annotation
-    exp = pytest.memote.experimental.growth[experiment]
+    name, exp = experiment
     expected = exp.data
     test = exp.evaluate(model)
     # Growth data sets need not use unique exchange reactions thus we use the
     # numeric index here to compute the confusion matrix.
-    ann["data"][experiment] = confusion_matrix(
+    ann["data"][name] = result = confusion_matrix(
         set(test.loc[test["growth"], "exchange"].index),
         set(expected.loc[expected["growth"], "exchange"].index),
         set(test.loc[~test["growth"], "exchange"].index),
         set(expected.loc[~expected["growth"], "exchange"].index)
     )
-    ann["metric"][experiment] = ann["data"][experiment]["ACC"]
-    ann["message"][experiment] = wrapper.fill(
+    ann["metric"][name] = result["ACC"]
+    ann["message"][name] = wrapper.fill(
         """Ideally, every model would show a perfect accuracy of 1. In
-        experiment '{}' the model has  {:.2}.""".format(
-            experiment, ann["data"][experiment]["MCC"]))
-    assert ann["data"][experiment]["ACC"] > threshold
+        name '{}' the model has  {:.2}.""".format(
+            name, result["MCC"]))
+    assert result["ACC"] > threshold

tests/test_for_experimental/test_for_config.py

@@ -76,7 +76,8 @@ def test_load_medium(filename, model, media, growth):
 def test_load_essentiality(filename, model, experiment):
     config = ExperimentConfiguration(join(DATA_PATH, filename))
     config.validate()
-    config.load_data(model)
+    config.load_medium(model)
+    config.load_essentiality(model)
     exp = config.essentiality[experiment]
     expected = exp.data
     expected.sort_values("gene", inplace=True)
@@ -91,7 +92,8 @@ def test_load_essentiality(filename, model, experiment):
 def test_load_growth(filename, model, experiment):
     config = ExperimentConfiguration(join(DATA_PATH, filename))
     config.validate()
-    config.load_data(model)
+    config.load_medium(model)
+    config.load_growth(model)
     exp = config.growth[experiment]
     expected = exp.data
     expected.sort_values("exchange", inplace=True)

tests/test_for_suite/test_for_cli/data/essentiality/core_deletion.csv

@@ -0,0 +1,5 @@
+gene,essential,comment
+b1779,True,
+b2416,True,
+b0356,False,
+b3213,False,

tests/test_for_suite/test_for_cli/data/growth/growth.csv

@@ -0,0 +1,5 @@
+exchange,uptake,growth,comment
+EX_glc__D_e,10,true,
+EX_fru_e,5,true,
+EX_pyr_e,10,true,
+EX_glc__D_e,0,false,

tests/test_for_suite/test_for_cli/data/media/glucose.csv

@@ -0,0 +1,8 @@
+exchange,uptake,comment
+EX_co2_e,1000.0,
+EX_h2o_e,1000.0,
+EX_h_e,1000.0,
+EX_nh4_e,1000.0,
+EX_o2_e,1000.0,
+EX_pi_e,1000.0,
+EX_glc__D_e,10.0,

tests/test_for_suite/test_for_cli/data/media/m9_minimal.csv

@@ -0,0 +1,7 @@
+exchange,uptake,comment
+EX_co2_e,1000.0,
+EX_h2o_e,1000.0,
+EX_h_e,1000.0,
+EX_nh4_e,1000.0,
+EX_o2_e,1000.0,
+EX_pi_e,1000.0,

tests/test_for_suite/test_for_cli/data/valid.yml

@@ -0,0 +1,16 @@
+version: "0.1"
+medium:
+  path: "media/"
+  definitions:
+    m9_minimal:
+    glucose:
+essentiality:
+  path: "essentiality/"
+  experiments:
+    core_deletion:
+      medium: "glucose"
+growth:
+  path: "growth/"
+  experiments:
+    core:
+      filename: "growth.csv"

tests/test_for_suite/test_for_cli/test_for_runner.py

@@ -21,7 +21,7 @@
 
 import os
 from builtins import str
-from os.path import exists, join
+from os.path import exists, join, dirname, pardir
 
 import pytest
 
@@ -43,6 +43,15 @@ def test_run_simple(runner, model_file):
     assert result.exit_code == 0
 
 
+def test_run_with_experimental_data(runner, model_file):
+    """Expect a simple run to function."""
+    result = runner.invoke(cli, [
+        "run", "--no-collect", "--ignore-git", "--experimental",
+        join(dirname(__file__), "data", "valid.yml"), model_file
+    ])
+    assert result.exit_code == 0
+
+
 def test_run_output(runner, model_file):
     """Expect a simple run to function."""
     output = model_file.split(".", 1)[0] + ".json"