Repository: yaml2sbml-dev/yaml2sbml (language: python)

Issue:
Error in time variable specification with YamlModel
```python
from tempfile import NamedTemporaryFile
from yaml2sbml import YamlModel
model = YamlModel()
model.set_time('t')
with NamedTemporaryFile(suffix='.xml') as f:
    model.write_to_sbml(f.name, over_write=True)
```
produces
```
Traceback (most recent call last):
    model.write_to_sbml(f.name, over_write=True)
  File "yaml2sbml/YamlModel.py", line 116, in write_to_sbml
    sbml_as_string = _parse_yaml_dict(reduced_model_dict)
  File "yaml2sbml/yaml2sbml.py", line 73, in _parse_yaml_dict
    _convert_yaml_blocks_to_sbml(model, yaml_dict)
  File "yaml2sbml/yaml2sbml.py", line 152, in _convert_yaml_blocks_to_sbml
    function_dict[block](model, yaml_dic[block])
  File "yaml2sbml/yaml2sbml/yaml2sbml.py", line 171, in _read_time_block
    if time_dic['variable'] == 'time':
TypeError: list indices must be integers or slices, not str
```

This can be fixed by changing the [specification of the time variable](https://github.com/yaml2sbml-dev/yaml2sbml/blob/master/yaml2sbml/YamlModel.py#L187) from
```python
        self._yaml_model['time'] = [time_variable]
```
to
```python
        self._yaml_model['time'] = {'variable': time_variable}
```
In the context of the YAML model file, this would be equivalent to changing
```yaml
time:
- t
```
to
```yaml
time:
  variable: t
```
Error in time variable specification with YamlModel
```python
from tempfile import NamedTemporaryFile
from yaml2sbml import YamlModel
model = YamlModel()
model.set_time('t')
with NamedTemporaryFile(suffix='.xml') as f:
    model.write_to_sbml(f.name, over_write=True)
```
produces
```
Traceback (most recent call last):
    model.write_to_sbml(f.name, over_write=True)
  File "yaml2sbml/YamlModel.py", line 116, in write_to_sbml
    sbml_as_string = _parse_yaml_dict(reduced_model_dict)
  File "yaml2sbml/yaml2sbml.py", line 73, in _parse_yaml_dict
    _convert_yaml_blocks_to_sbml(model, yaml_dict)
  File "yaml2sbml/yaml2sbml.py", line 152, in _convert_yaml_blocks_to_sbml
    function_dict[block](model, yaml_dic[block])
  File "yaml2sbml/yaml2sbml/yaml2sbml.py", line 171, in _read_time_block
    if time_dic['variable'] == 'time':
TypeError: list indices must be integers or slices, not str
```

This can be fixed by changing the [specification of the time variable](https://github.com/yaml2sbml-dev/yaml2sbml/blob/master/yaml2sbml/YamlModel.py#L187) from
```python
        self._yaml_model['time'] = [time_variable]
```
to
```python
        self._yaml_model['time'] = {'variable': time_variable}
```
In the context of the YAML model file, this would be equivalent to changing
```yaml
time:
- t
```
to
```yaml
time:
  variable: t
```


Discussion / hints:
The problem, that I currently see, is that this is not caught by `model.validate_yaml()`,  so I will have a look into this as well :) 
The problem, that I currently see, is that this is not caught by `model.validate_yaml()`,  so I will have a look into this as well :) 

Fix patch:
diff --git a/yaml2sbml/YamlModel.py b/yaml2sbml/YamlModel.py
index c0ff853..7d26ce3 100644
--- a/yaml2sbml/YamlModel.py
+++ b/yaml2sbml/YamlModel.py
@@ -17,7 +17,7 @@ def __init__(self):
         """
         Set up yaml model.
         """
-        self._yaml_model = {'time': None,
+        self._yaml_model = {'time': {},
                             'odes': [],
                             'parameters': [],
                             'assignments': [],
@@ -174,7 +174,7 @@ def _get_reduced_model_dict(self) -> dict:
 
         for (key, val) in self._yaml_model.items():
             if val:
-               reduced_model_dict[key] = copy.deepcopy(val)
+                reduced_model_dict[key] = copy.deepcopy(val)
 
         return reduced_model_dict
 
@@ -184,10 +184,13 @@ def is_set_time(self):
 
     def set_time(self,
                  time_variable: str):
-        self._yaml_model['time'] = [time_variable]
+        self._yaml_model['time'] = {'variable': time_variable}
 
     def get_time(self):
-        return self._yaml_model['time'][0]
+        if self.is_set_time():
+            return self._yaml_model['time']['variable']
+        else:
+            return None
 
     # functions adding a value
     def add_parameter(self,
diff --git a/yaml2sbml/yaml_schema.yaml b/yaml2sbml/yaml_schema.yaml
index c4f5f11..b3640a8 100644
--- a/yaml2sbml/yaml_schema.yaml
+++ b/yaml2sbml/yaml_schema.yaml
@@ -6,11 +6,13 @@ description: yaml2sbml file format
 properties:
 
   time:
+    type: object
     items:
       variable:
         type: string
         description: defines a time variable, in case the right hand side of the ODE is time-dependent.
-
+    required:
+      - variable
 
   parameters:
     type: array


Test patch:
diff --git a/tests/test_YamlModel.py b/tests/test_YamlModel.py
index a02407f..43842b5 100644
--- a/tests/test_YamlModel.py
+++ b/tests/test_YamlModel.py
@@ -254,6 +254,21 @@ def test_condition(self):
         model.delete_condition(condition_id)
         self.assertListEqual(model.get_condition_ids(), [])
 
+    def test_valid_model(self):
+        """
+        Tests, whether the resulting models are valid.
+        """
+        model = YamlModel()
+
+        model.set_time('t')
+        model.add_ode(state_id='x',
+                      right_hand_side='k_1 * x + t',
+                      initial_value=0)
+        model.add_parameter(parameter_id='k_1',
+                            nominal_value=1)
+
+        model.validate_model()
+
 
 if __name__ == '__main__':
     suite = unittest.TestSuite()
diff --git a/tests/test_yaml2sbml/ode_input_invalid_time_1.yaml b/tests/test_yaml2sbml/ode_input_invalid_time_1.yaml
new file mode 100644
index 0000000..08acc01
--- /dev/null
+++ b/tests/test_yaml2sbml/ode_input_invalid_time_1.yaml
@@ -0,0 +1,9 @@
+time:
+  - t
+odes:
+- initialValue: 0
+  rightHandSide: k_1 * x + t
+  stateId: x
+parameters:
+- nominalValue: 1
+  parameterId: k_1
diff --git a/tests/test_yaml2sbml/ode_input_invalid_time_2.yaml b/tests/test_yaml2sbml/ode_input_invalid_time_2.yaml
new file mode 100644
index 0000000..dedcf75
--- /dev/null
+++ b/tests/test_yaml2sbml/ode_input_invalid_time_2.yaml
@@ -0,0 +1,9 @@
+time:
+  - variable: t
+odes:
+- initialValue: 0
+  rightHandSide: k_1 * x + t
+  stateId: x
+parameters:
+- nominalValue: 1
+  parameterId: k_1
diff --git a/tests/test_yaml_validation.py b/tests/test_yaml_validation.py
index 19cce2f..0bebd4d 100644
--- a/tests/test_yaml_validation.py
+++ b/tests/test_yaml_validation.py
@@ -36,6 +36,18 @@ def test_validate_yaml_empty_section(self):
         with self.assertRaises(ValidationError):
             validate_yaml(file_in)
 
+    def test_catch_invalid_time_block_missing_variable_key(self):
+        # time block without kew word "variable"
+        file_in = os.path.join(self.test_folder, 'ode_input_invalid_time_1.yaml')
+        with self.assertRaises(ValidationError):
+            validate_yaml(file_in)
+
+    def test_catch_invalid_time_block_as_array(self):
+        # time block as array instead of single object
+        file_in = os.path.join(self.test_folder, 'ode_input_invalid_time_2.yaml')
+        with self.assertRaises(ValidationError):
+            validate_yaml(file_in)
+
 
 if __name__ == '__main__':
     suite = unittest.TestSuite()