Fix invalid component handling (#431)

Previously, if one added an invalid parameter, rule etc. that generated
an exception in `__init__`, (e.g. a validation error), the component
was still added to the model. In interactive mode, this is annoying, as
the model has to be re-instantiated. Example:

    Model()
    Parameter('a', 'invalid_value')  # raises ValueError
    assert len(model.parameters) == 0  # fixed in this PR
    Parameter('a', 1.0)  # should succeed

This PR fixes this situation by adding the Component to the Model
(by calling `Component.__init__()`) last instead of first.

pysb/core.py

@@ -263,8 +263,6 @@ class Monomer(Component):
     alphanumeric or underscores.
     """
     def __init__(self, name, sites=None, site_states=None, _export=True):
-        Component.__init__(self, name, _export)
-
         # Create default empty containers.
         if sites is None:
             sites = []
@@ -304,6 +302,7 @@ def __init__(self, name, sites=None, site_states=None, _export=True):
 
         self.sites = list(sites)
         self.site_states = site_states
+        Component.__init__(self, name, _export)
 
     def __call__(self, conditions=None, **kwargs):
         """
@@ -1100,8 +1099,8 @@ def __getnewargs__(self):
         return (self.name, self.value, False)
 
     def __init__(self, name, value=0.0, _export=True):
-        Component.__init__(self, name, _export)
         self.value = float(value)
+        Component.__init__(self, name, _export)
 
     def get_value(self):
         return self.value
@@ -1157,7 +1156,6 @@ class Compartment(Component):
     """
 
     def __init__(self, name, parent=None, dimension=3, size=None, _export=True):
-        Component.__init__(self, name, _export)
         if parent != None and isinstance(parent, Compartment) == False:
             raise Exception("parent must be a predefined Compartment or None")
         #FIXME: check for only ONE "None" parent? i.e. only one compartment can have a parent None?
@@ -1166,6 +1164,7 @@ def __init__(self, name, parent=None, dimension=3, size=None, _export=True):
         self.parent = parent
         self.dimension = dimension
         self.size = size
+        Component.__init__(self, name, _export)
 
     def __repr__(self):
         return '%s(name=%s, parent=%s, dimension=%s, size=%s)' % (
@@ -1214,7 +1213,6 @@ class Rule(Component):
     def __init__(self, name, rule_expression, rate_forward, rate_reverse=None,
                  delete_molecules=False, move_connected=False,
                  _export=True):
-        Component.__init__(self, name, _export)
         if not isinstance(rule_expression, RuleExpression):
             raise Exception("rule_expression is not a RuleExpression object")
         validate_expr(rate_forward, "forward rate")
@@ -1237,7 +1235,9 @@ def __init__(self, name, rule_expression, rate_forward, rate_reverse=None,
             for cp in rp.complex_patterns:
                 if not cp.is_concrete():
                     raise ValueError('Product {} of synthesis rule {} is not '
-                                     'concrete'.format(cp, self.name))
+                                     'concrete'.format(cp, name))
+
+        Component.__init__(self, name, _export)
 
     def is_synth(self):
         """Return a bool indicating whether this is a synthesis rule."""
@@ -1391,11 +1391,11 @@ def __getnewargs__(self):
         return (self.name, self.expr, False)
 
     def __init__(self, name, expr, _export=True):
-        Component.__init__(self, name, _export)
         if not isinstance(expr, sympy.Expr):
             raise ValueError('An Expression can only be created from a '
                              'sympy.Expr object')
         self.expr = expr
+        Component.__init__(self, name, _export)
 
     def expand_expr(self, expand_observables=False):
         """Return expr rewritten in terms of terminal symbols only."""

pysb/tests/test_core.py

@@ -378,3 +378,44 @@ def test_rulepattern_match_none_against_state():
     # so this should be a valid rule pattern
     A(phospho=None) + A(phospho=None) >> A(phospho=1) % A(phospho=1)
 
+
+@with_model
+def test_invalid_rule():
+    Monomer('A')
+    assert_raises(ExpressionError, Rule, 'r1', None >> A(), 1.0)
+    assert len(model.rules) == 0
+
+    Parameter('kf', 1.0)
+    assert_raises(Exception, Rule, 'r1', 'invalid_rule_expr', kf)
+    assert len(model.rules) == 0
+
+
+@with_model
+def test_invalid_expression():
+    assert_raises(ValueError, Expression, 'e1', 'invalid_expr')
+    assert len(model.expressions) == 0
+
+
+@with_model
+def test_invalid_monomer_name():
+    assert_raises(ValueError, Monomer, 'a', 123)
+    assert len(model.monomers) == 0
+
+
+@with_model
+def test_invalid_parameter():
+    assert_raises(ValueError, Parameter, 'a', 'invalid_value')
+    assert len(model.parameters) == 0
+
+
+@with_model
+def test_invalid_compartment():
+    assert_raises(Exception, Compartment, 'c1', 'invalid_parent')
+    assert len(model.compartments) == 0
+
+
+@with_model
+def test_invalid_observable():
+    assert_raises(InvalidReactionPatternException,
+                  Observable, 'o1', 'invalid_pattern')
+    assert len(model.observables) == 0