Repository: pybamm-team/PyBaMM

Issue:
Tests fail when scikits not installed
**Describe the bug**
All tests fail when scikits not installed, e.g.
```
======================================================================
ERROR: test_discretisations.test_discretisation (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_discretisations.test_discretisation
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/loader.py", line 428, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.5/unittest/loader.py", line 369, in _get_module_from_name
    __import__(name)
  File "/home/ivan/workspace/PyBaMM/tests/unit/test_discretisations/test_discretisation.py", line 4, in <module>
    import pybamm
  File "/home/ivan/workspace/PyBaMM/pybamm/__init__.py", line 174, in <module>
    from .solvers.scikits_dae_solver import ScikitsDaeSolver
  File "/home/ivan/workspace/PyBaMM/pybamm/solvers/scikits_dae_solver.py", line 102, in <module>
    class JacobianFunctionIDA(ida.IDA_JacRhsFunction):
NameError: name 'ida' is not defined
```



Discussion:

Patch:
diff --git a/README.md b/README.md
index 9f31488f5..a6857b96d 100644
--- a/README.md
+++ b/README.md
@@ -14,6 +14,8 @@ PyBaMM comes with a number of [detailed examples](examples/README.md), hosted he
 
 ## How can I install PyBaMM?
 
+The safest way to install PyBaMM is to do so within a virtual environment ([introduction to virtual environments](https://realpython.com/python-virtual-environments-a-primer/))
+
 You'll need the following requirements:
 
 - Python 3.5+
diff --git a/pybamm/__init__.py b/pybamm/__init__.py
index d86a8349f..4ee569b8e 100644
--- a/pybamm/__init__.py
+++ b/pybamm/__init__.py
@@ -84,7 +84,7 @@ from .expression_tree.unary_operators import (
     div,
     surf,
     Diagonal,
-    integrate
+    integrate,
 )
 from .expression_tree.parameter import Parameter, FunctionParameter
 from .expression_tree.broadcasts import Broadcast, NumpyBroadcast
diff --git a/pybamm/models/base_models.py b/pybamm/models/base_models.py
index 5196adc57..df43b6926 100644
--- a/pybamm/models/base_models.py
+++ b/pybamm/models/base_models.py
@@ -111,7 +111,10 @@ class BaseModel(object):
             "negative particle": pybamm.FiniteVolume,
             "positive particle": pybamm.FiniteVolume,
         }
-        self.default_solver = pybamm.ScikitsOdeSolver()
+        try:
+            self.default_solver = pybamm.ScikitsOdeSolver()
+        except ImportError:
+            self.default_solver = pybamm.ScipySolver()
 
     def _set_dict(self, dict, name):
         """
diff --git a/pybamm/solvers/scikits_dae_solver.py b/pybamm/solvers/scikits_dae_solver.py
index c0ba707cd..9b9496dd1 100644
--- a/pybamm/solvers/scikits_dae_solver.py
+++ b/pybamm/solvers/scikits_dae_solver.py
@@ -17,6 +17,10 @@ if scikits_odes_spec is not None:
         scikits_odes_spec.loader.exec_module(scikits_odes)
         from scikits.odes.sundials import ida
 
+        jac_class = ida.IDA_JacRhsFunction
+else:
+    jac_class = object
+
 
 class ScikitsDaeSolver(pybamm.DaeSolver):
     """Solve a discretised model, using scikits.odes.
@@ -99,7 +103,7 @@ class ScikitsDaeSolver(pybamm.DaeSolver):
         return sol.values.t, np.transpose(sol.values.y)
 
 
-class JacobianFunctionIDA(ida.IDA_JacRhsFunction):
+class JacobianFunctionIDA(jac_class):
     def set_jacobian(self, mass_matrix, jacobian):
         """
         Sets the user supplied mass matrix and Jacobian function for the DAE model.
diff --git a/pybamm/solvers/scikits_ode_solver.py b/pybamm/solvers/scikits_ode_solver.py
index 2728d2562..e4ffac8e1 100644
--- a/pybamm/solvers/scikits_ode_solver.py
+++ b/pybamm/solvers/scikits_ode_solver.py
@@ -17,6 +17,10 @@ if scikits_odes_spec is not None:
         scikits_odes_spec.loader.exec_module(scikits_odes)
         from scikits.odes.sundials import cvode
 
+        jac_class = cvode.CV_JacRhsFunction
+else:
+    jac_class = object
+
 
 class ScikitsOdeSolver(pybamm.OdeSolver):
     """Solve a discretised model, using scikits.odes.
@@ -95,7 +99,7 @@ class ScikitsOdeSolver(pybamm.OdeSolver):
         return sol.values.t, np.transpose(sol.values.y)
 
 
-class JacobianFunctionCV(cvode.CV_JacRhsFunction):
+class JacobianFunctionCV(jac_class):
     def set_jacobian(self, jacobian):
         """
         Sets the user supplied Jacobian function for the ODE model.


Test patch:
diff --git a/tests/unit/test_models/test_lead_acid/test_newman_tiedemann.py b/tests/unit/test_models/test_lead_acid/test_newman_tiedemann.py
index 6981e0049..d0df184a5 100644
--- a/tests/unit/test_models/test_lead_acid/test_newman_tiedemann.py
+++ b/tests/unit/test_models/test_lead_acid/test_newman_tiedemann.py
@@ -4,12 +4,14 @@
 from __future__ import absolute_import, division
 from __future__ import print_function, unicode_literals
 import pybamm
+from pybamm.solvers.scikits_ode_solver import scikits_odes_spec
 import tests
 
 import unittest
 import numpy as np
 
 
+@unittest.skipIf(scikits_odes_spec is None, "scikits.odes not installed")
 class TestLeadAcidNewmanTiedemann(unittest.TestCase):
     def test_basic_processing(self):
         model = pybamm.lead_acid.NewmanTiedemann()
diff --git a/tests/unit/test_models/test_lithium_ion/test_lithium_ion_dfn.py b/tests/unit/test_models/test_lithium_ion/test_lithium_ion_dfn.py
index 24fd18740..a51917da6 100644
--- a/tests/unit/test_models/test_lithium_ion/test_lithium_ion_dfn.py
+++ b/tests/unit/test_models/test_lithium_ion/test_lithium_ion_dfn.py
@@ -4,12 +4,14 @@
 from __future__ import absolute_import, division
 from __future__ import print_function, unicode_literals
 import pybamm
+from pybamm.solvers.scikits_ode_solver import scikits_odes_spec
 import tests
 
 import numpy as np
 import unittest
 
 
+@unittest.skipIf(scikits_odes_spec is None, "scikits.odes not installed")
 class TestDFN(unittest.TestCase):
     def test_basic_processing(self):
         model = pybamm.lithium_ion.DFN()
diff --git a/tests/unit/test_models/test_submodels/test_electrode.py b/tests/unit/test_models/test_submodels/test_electrode.py
index 2f2f2088b..4ddfcac0d 100644
--- a/tests/unit/test_models/test_submodels/test_electrode.py
+++ b/tests/unit/test_models/test_submodels/test_electrode.py
@@ -4,6 +4,7 @@
 from __future__ import absolute_import, division
 from __future__ import print_function, unicode_literals
 import pybamm
+from pybamm.solvers.scikits_ode_solver import scikits_odes_spec
 import tests
 import numpy as np
 import numbers
@@ -11,6 +12,7 @@ import numbers
 import unittest
 
 
+@unittest.skipIf(scikits_odes_spec is None, "scikits.odes not installed")
 class TestOhm(unittest.TestCase):
     def test_basic_processing(self):
         # Parameters
diff --git a/tests/unit/test_models/test_submodels/test_electrolyte_current.py b/tests/unit/test_models/test_submodels/test_electrolyte_current.py
index 62048966b..84d256c5b 100644
--- a/tests/unit/test_models/test_submodels/test_electrolyte_current.py
+++ b/tests/unit/test_models/test_submodels/test_electrolyte_current.py
@@ -4,6 +4,7 @@
 from __future__ import absolute_import, division
 from __future__ import print_function, unicode_literals
 import pybamm
+from pybamm.solvers.scikits_ode_solver import scikits_odes_spec
 import tests
 import numbers
 import numpy as np
@@ -11,6 +12,7 @@ import numpy as np
 import unittest
 
 
+@unittest.skipIf(scikits_odes_spec is None, "scikits.odes not installed")
 class TestMacInnesStefanMaxwell(unittest.TestCase):
     def test_basic_processing(self):
         # Parameters