Allow usage of config dict instead of file only (#108)

* Allow usage of config dict instead of file only

This allows consumer codes to use a different input file config format and still being able to convert it into dict and pass it to  Circuit/Simulation classes.

bluepysnap/config.py

@@ -35,29 +35,34 @@ class Config(object):
         https://github.com/AllenInstitute/sonata/blob/master/docs/SONATA_DEVELOPER_GUIDE.md#network_config
     """
 
-    def __init__(self, filepath):
+    def __init__(self, config):
         """Initializes a Config object from a path to the actual config.
 
         Args:
-            filepath (str): Path the SONATA configuration file.
+            config (str/dict): Path to the SONATA configuration file or dict containing the config.
 
         Returns:
              Config: A Config object.
         """
-        configdir = str(Path(filepath).parent.resolve())
-        content = utils.load_json(str(filepath))
+        if isinstance(config, dict):
+            content = config.copy()
+            configdir = None
+        else:
+            configdir = str(Path(config).parent.resolve())
+            content = utils.load_json(str(config))
         self.manifest = Config._resolve_manifest(content.pop('manifest', {}), configdir)
         self.content = content
 
     @staticmethod
     def _resolve_manifest(manifest, configdir):
         result = manifest.copy()
 
-        assert '${configdir}' not in result
-        result['${configdir}'] = configdir
-
         for k, v in six.iteritems(result):
-            if v.startswith('.'):
+            if not isinstance(v, six.string_types):
+                raise BluepySnapError('{} should be a string value.'.format(v))
+            if not Path(v).is_absolute() and not v.startswith("$"):
+                if configdir is None:
+                    raise BluepySnapError("Dictionary config with relative paths is not allowed.")
                 result[k] = str(Path(configdir, v).resolve())
 
         while True:
@@ -75,9 +80,8 @@ def _resolve_manifest(manifest, configdir):
             if not update:
                 break
 
-        for k, v in result.items():
-            if not v.startswith('/'):
-                raise BluepySnapError("{} cannot be resolved as an abs path.".format(k))
+        assert '${configdir}' not in result
+        result['${configdir}'] = configdir
 
         return result
 
@@ -93,8 +97,11 @@ def _resolve_string(self, value):
                 raise BluepySnapError("Misplaced anchors in : {}."
                                       "Please verify your '$' usage.".format(value))
             return str(Path(*vs))
+        # only way to know if value is a relative path or a normal string
         elif value.startswith('.'):
-            return str(Path(self.manifest['${configdir}'], value).resolve())
+            if self.manifest['${configdir}'] is not None:
+                return str(Path(self.manifest['${configdir}'], value).resolve())
+            raise BluepySnapError("Dictionary config with relative paths is not allowed.")
         else:
             # we cannot know if a string is a path or not if it does not contain anchor or .
             return value

bluepysnap/simulation.py

@@ -17,34 +17,29 @@
 """Simulation access."""
 
 from cached_property import cached_property
-from pathlib2 import Path
 
 from bluepysnap.node_sets import NodeSets
 from bluepysnap.config import Config
 from bluepysnap.exceptions import BluepySnapError
 
-from bluepysnap import utils
 
-
-def _resolve_config(filepath):
+def _resolve_config(config):
     """Resolve the config file if global ('network' and 'simulation' keys).
 
     Args:
-        filepath (str): the path to the configuration file.
+        config (str/dict): the path to the configuration file or a dict containing the config.
 
     Returns:
         dict: the complete simulation config file.
     """
-    filepath = Path(filepath)
-    content = utils.load_json(str(filepath))
-    parent = filepath.parent
+    content = Config(config).resolve()
     if "simulation" in content and "network" in content:
-        simulation_path = parent / content["simulation"]
+        simulation_path = content["simulation"]
         res = Config(str(simulation_path)).resolve()
         if "network" not in res:
-            res["network"] = str(parent / content["network"])
+            res["network"] = str(content["network"])
         return res
-    return Config(str(filepath)).resolve()
+    return content
 
 
 def _collect_frame_reports(sim):

setup.py

@@ -44,7 +44,8 @@ def __init__(self, *args, **kwargs):
         'cached_property>=1.0',
         'functools32;python_version<"3.2"',
         'h5py>=2.2,<3',
-        'libsonata>=0.1.4',
+        'libsonata<=0.1.4;python_version<"3.6"',
+        'libsonata>=0.1.6;python_version>="3.6"',
         'neurom>=1.3',
         'numpy>=1.8',
         'pandas>=0.17',

tests/test_config.py

@@ -1,5 +1,6 @@
 import pytest
 from pathlib2 import Path
+import json
 
 import bluepysnap.config as test_module
 from bluepysnap.exceptions import BluepySnapError
@@ -57,6 +58,21 @@ def test_parse():
         actual = test_module.Config.parse(config_path)
         assert actual["something"] == str(Path(config_path.parent) / 'something' / 'else')
 
+    # check resolution with $ in a middle of the words
+    with copy_config() as config_path:
+        with edit_config(config_path) as config:
+            config["something"] = "$COMPONENT_DIR/somet$hing/else"
+        actual = test_module.Config.parse(config_path)
+        assert actual["something"] == str(Path(config_path.parent) / 'somet$hing' / 'else')
+
+    # check resolution with relative path without "." in the manifest
+    with copy_config() as config_path:
+        with edit_config(config_path) as config:
+            config["manifest"]["$NOPOINT"] = "nopoint"
+            config["components"]["other"] = "$NOPOINT/other"
+        actual = test_module.Config.parse(config_path)
+        assert actual["components"]["other"] == str(Path(config_path.parent) / 'nopoint' / 'other')
+
     # check resolution for non path objects
     with copy_config() as config_path:
         with edit_config(config_path) as config:
@@ -82,37 +98,42 @@ def test_parse():
 
 
 def test_bad_manifest():
-    # not abs path in
-    with copy_config() as config_path:
-        with edit_config(config_path) as config:
-            config["manifest"]["$BASE_DIR"] = "not_absolute"
-        with pytest.raises(BluepySnapError):
-            test_module.Config.parse(config_path)
-
+    # 2 anchors would result in the absolute path of the last one : misleading
     with copy_config() as config_path:
         with edit_config(config_path) as config:
             config["manifest"]["$COMPONENT_DIR"] = "$BASE_DIR/$NETWORK_DIR"
         with pytest.raises(BluepySnapError):
             test_module.Config.parse(config_path)
 
+    # same but not in the manifest
     with copy_config() as config_path:
         with edit_config(config_path) as config:
             config["components"]["other"] = "$COMPONENT_DIR/$BASE_DIR"
         with pytest.raises(BluepySnapError):
             test_module.Config.parse(config_path)
 
+    # relative path with an anchor in the middle is not allowed this breaks the purpose of the
+    # anchors (they are not just generic placeholders)
     with copy_config() as config_path:
         with edit_config(config_path) as config:
             config["components"]["other"] = "something/$COMPONENT_DIR/"
         with pytest.raises(BluepySnapError):
             test_module.Config.parse(config_path)
 
+    # abs path with an anchor in the middle is not allowed
     with copy_config() as config_path:
         with edit_config(config_path) as config:
             config["components"]["other"] = "/something/$COMPONENT_DIR/"
         with pytest.raises(BluepySnapError):
             test_module.Config.parse(config_path)
 
+    # unknown anchor
+    with copy_config() as config_path:
+        with edit_config(config_path) as config:
+            config["components"]["other"] = "$UNKNOWN/something/"
+        with pytest.raises(KeyError):
+            test_module.Config.parse(config_path)
+
 
 def test_simulation_config():
     actual = test_module.Config.parse(
@@ -123,3 +144,36 @@ def test_simulation_config():
     assert actual["mechanisms_dir"] == str(Path(TEST_DATA_DIR / "../shared_components_mechanisms").resolve())
     assert actual["conditions"]["celsius"] == 34.0
     assert actual["conditions"]["v_init"] == -80
+
+
+def test_dict_config():
+    config = json.load(open(str(TEST_DATA_DIR / 'circuit_config.json'), "r"))
+    # there are relative paths in the manifest you cannot resolve if you are using a dict
+    with pytest.raises(BluepySnapError):
+        test_module.Config(config)
+
+    config["manifest"]["$NETWORK_DIR"] = str(TEST_DATA_DIR)
+    config["manifest"]["$BASE_DIR"] = str(TEST_DATA_DIR)
+
+    expected = test_module.Config.parse(str(TEST_DATA_DIR / 'circuit_config.json'))
+    assert test_module.Config(config).resolve() == expected
+
+    config = json.load(open(str(TEST_DATA_DIR / 'simulation_config.json'), "r"))
+    # there are relative paths in the manifest you cannot resolve if you are using a dict and
+    # the field "mechanisms_dir" is using a relative path
+    with pytest.raises(BluepySnapError):
+        test_module.Config(config)
+
+    config["manifest"]["$OUTPUT_DIR"] = str(TEST_DATA_DIR / "reporting")
+    config["manifest"]["$INPUT_DIR"] = str(TEST_DATA_DIR)
+
+    # the field "mechanisms_dir" is still using a relative path
+    with pytest.raises(BluepySnapError):
+        test_module.Config(config).resolve()
+
+    # does not allow Paths as values
+    with pytest.raises(BluepySnapError):
+        config = json.load(open(str(TEST_DATA_DIR / 'circuit_config.json'), "r"))
+        config["manifest"]["$NETWORK_DIR"] = TEST_DATA_DIR
+        config["manifest"]["$BASE_DIR"] = TEST_DATA_DIR
+        test_module.Config.parse(config)

tests/test_simulation.py

@@ -1,4 +1,5 @@
 import pytest
+import json
 
 from bluepysnap.exceptions import BluepySnapError
 import bluepysnap.simulation as test_module
@@ -13,7 +14,7 @@
 def test_all():
     simulation = test_module.Simulation(str(TEST_DATA_DIR / 'simulation_config.json'))
     assert simulation.config["network"] == str(TEST_DATA_DIR / 'circuit_config.json')
-    assert sorted(list(simulation.circuit.nodes)) == ['default', 'default2']
+    assert set(simulation.circuit.nodes) == {'default', 'default2'}
     assert list(simulation.circuit.edges) == ['default']
 
     assert simulation.run == {"tstop": 1000.0, "dt": 0.01, "spike_threshold": -15,
@@ -30,16 +31,16 @@ def test_all():
     assert isinstance(simulation.spikes, SpikeReport)
     assert isinstance(simulation.spikes["default"], PopulationSpikeReport)
 
-    assert sorted(list(simulation.reports)) == sorted(list(['soma_report', 'section_report']))
+    assert set(simulation.reports) == {'soma_report', 'section_report'}
     assert isinstance(simulation.reports['soma_report'], SomaReport)
     assert isinstance(simulation.reports['section_report'], CompartmentReport)
 
     rep = simulation.reports['soma_report']
-    assert sorted(list(rep.population_names)) == ["default", "default2"]
+    assert set(rep.population_names) == {"default", "default2"}
     assert isinstance(rep['default'], PopulationSomaReport)
 
     rep = simulation.reports['section_report']
-    assert sorted(list(rep.population_names)) == ["default", "default2"]
+    assert set(rep.population_names) == {"default", "default2"}
     assert isinstance(rep['default'], PopulationCompartmentReport)
 
 
@@ -60,11 +61,11 @@ def test_unknown_report():
 def test__resolve_config():
     simulation = test_module.Simulation(str(TEST_DATA_DIR / 'config.json'))
     assert simulation.config["network"] == str(TEST_DATA_DIR / 'circuit_config.json')
-    assert sorted(list(simulation.circuit.nodes)) == ['default', 'default2']
+    assert set(simulation.circuit.nodes) == {'default', 'default2'}
 
     simulation = test_module.Simulation(str(TEST_DATA_DIR / 'config_sim_no_network.json'))
     assert simulation.config["network"] == str(TEST_DATA_DIR / 'circuit_config.json')
-    assert sorted(list(simulation.circuit.nodes)) == ['default', 'default2']
+    assert set(simulation.circuit.nodes) == {'default', 'default2'}
 
 
 def test_no_network_config():
@@ -80,3 +81,32 @@ def test_no_node_set():
     # replace the _config dict with random one that does not contain "node_sets_file" key
     simulation._config = {"key": "value"}
     assert simulation.node_sets == {}
+
+
+def test__resolve_config_dict():
+    input_dict = {
+        "network": "./circuit_config.json",
+        "simulation": "./simulation_config.json"
+    }
+    with pytest.raises(BluepySnapError):
+        test_module.Simulation(input_dict)
+
+    input_dict = {
+        "network": str(TEST_DATA_DIR / "./circuit_config.json"),
+        "simulation": str(TEST_DATA_DIR / "./simulation_config.json")
+    }
+    simulation = test_module.Simulation(input_dict)
+    assert simulation.config["network"] == str(TEST_DATA_DIR / 'circuit_config.json')
+    assert set(simulation.circuit.nodes) == {'default', 'default2'}
+
+    input_dict = json.load(open(str(TEST_DATA_DIR / "./simulation_config.json"), "r"))
+    with pytest.raises(BluepySnapError):
+        test_module.Simulation(input_dict)
+
+    input_dict['mechanisms_dir'] = "/abspath"
+    input_dict["manifest"]["$OUTPUT_DIR"] = str(TEST_DATA_DIR / "reporting")
+    input_dict["manifest"]["$INPUT_DIR"] = str(TEST_DATA_DIR)
+
+    simulation = test_module.Simulation(input_dict)
+    assert simulation.config["network"] == str(TEST_DATA_DIR / 'circuit_config.json')
+    assert set(simulation.circuit.nodes) == {'default', 'default2'}