Repository: ELIFE-ASU/Neet

Issue:
Death to Generators
Generators are great when you may have to iterate over computations that are too large to store in memory. This is certainly a possibility for much of Neet's algorithms. It was for this reason that @dglmoore over-engineered Neet's functions to `yield` instead of `return` where possible.

While it may seem like this is a good thing, it is really just cumbersome for users, implementers and maintainers. The problem it aimed to solve is a non-issue as most of the time there are computational limits other than memory that come into play around the same time as memory issues.

We should refactor to use lists in place of generators unless it makes sense to do otherwise.

Discussion:

Patch:
diff --git a/neet/synchronous.py b/neet/synchronous.py
index 1c6ed88..8f59454 100644
--- a/neet/synchronous.py
+++ b/neet/synchronous.py
@@ -35,7 +35,7 @@ def trajectory(net, state, timesteps=1, encode=False):
     :param state: the network state
     :param timesteps: the number of steps in the trajectory
     :param encode: encode the states as integers
-    :yields: the next state in the trajectory
+    :returns: the trajectory as a list
     :raises TypeError: if net is not a network
     :raises ValueError: if ``timesteps < 1``
     """
@@ -44,6 +44,7 @@ def trajectory(net, state, timesteps=1, encode=False):
     if timesteps < 1:
         raise ValueError("number of steps must be positive, non-zero")
 
+    traj = []
     state = copy.copy(state)
     if encode:
         if is_fixed_sized(net):
@@ -51,23 +52,24 @@ def trajectory(net, state, timesteps=1, encode=False):
         else:
             state_space = net.state_space(len(state))
 
-        yield state_space._unsafe_encode(state)
+        traj.append(state_space._unsafe_encode(state))
 
         net.update(state)
-        yield state_space._unsafe_encode(state)
+        traj.append(state_space._unsafe_encode(state))
 
         for _ in range(1,timesteps):
             net._unsafe_update(state)
-            yield state_space._unsafe_encode(state)
+            traj.append(state_space._unsafe_encode(state))
     else:
-        yield copy.copy(state)
+        traj.append(copy.copy(state))
 
         net.update(state)
-        yield copy.copy(state)
+        traj.append(copy.copy(state))
 
         for _ in range(1, timesteps):
             net._unsafe_update(state)
-            yield copy.copy(state)
+            traj.append(copy.copy(state))
+    return traj
 
 def transitions(net, size=None, encode=False):
     """
@@ -94,7 +96,7 @@ def transitions(net, size=None, encode=False):
     :param net: the network
     :param size: the size of the network (``None`` if fixed sized)
     :param encode: encode the states as integers
-    :yields: the one-state transitions
+    :returns: the one-state transitions as an array
     :raises TypeError: if ``net`` is not a network
     :raises ValueError: if ``net`` is fixed sized and ``size`` is not ``None``
     :raises ValueError: if ``net`` is not fixed sized and ``size`` is ``None``
@@ -111,12 +113,15 @@ def transitions(net, size=None, encode=False):
             raise ValueError("size must not be None for variable sized networks")
         state_space = net.state_space(size)
 
+    trans = []
     for state in state_space:
         net._unsafe_update(state)
         if encode:
-            yield state_space._unsafe_encode(state)
+            trans.append(state_space._unsafe_encode(state))
         else:
-            yield state
+            trans.append(state)
+
+    return trans
 
 def transition_graph(net, size=None):
     """
@@ -172,15 +177,14 @@ def attractors(net, size=None):
 
     :param net: the network or the transition graph
     :param size: the size of the network (``None`` if fixed sized)
-    :returns: a generator of attractors
+    :returns: a list of attractor cycles
     :raises TypeError: if ``net`` is not a network or a ``networkx.DiGraph``
     :raises ValueError: if ``net`` is fixed sized and ``size`` is not ``None``
     :raises ValueError: if ``net`` is a transition graph and ``size`` is not ``None``
     :raises ValueError: if ``net`` is not fixed sized and ``size`` is ``None``
     """
     if isinstance(net, nx.DiGraph):
-        for attr in nx.simple_cycles(net):
-            yield attr
+        return list(nx.simple_cycles(net))
     elif not is_network(net):
         raise TypeError("net must be a network or a networkx DiGraph")
     elif is_fixed_sized(net) and size is not None:
@@ -188,6 +192,7 @@ def attractors(net, size=None):
     elif not is_fixed_sized(net) and size is None:
         raise ValueError("variable sized networks require a size")
     else:
+        cycles = []
         # Get the state transitions
         # (array of next state indexed by current state)
         trans = list(transitions(net, size=size, encode=True))
@@ -259,7 +264,8 @@ def attractors(net, size=None):
 
             # Yield the cycle if we found one
             if len(cycle) != 0:
-                yield cycle
+                cycles.append(cycle)
+    return cycles
 
 def basins(net, size=None):
     """


Test patch:
diff --git a/test/test_synchronous.py b/test/test_synchronous.py
index ec1bb85..eed14a2 100644
--- a/test/test_synchronous.py
+++ b/test/test_synchronous.py
@@ -20,13 +20,13 @@ class TestSynchronous(unittest.TestCase):
         ``trajectory`` should raise a type error if ``net`` is not a network
         """
         with self.assertRaises(TypeError):
-            list(trajectory(5, [1, 2, 3]))
+            trajectory(5, [1, 2, 3])
 
         with self.assertRaises(TypeError):
-            list(trajectory(MockObject(), [1, 2, 3]))
+            trajectory(MockObject(), [1, 2, 3])
 
         with self.assertRaises(TypeError):
-            list(trajectory(MockFixedSizedNetwork, [1, 2, 3]))
+            trajectory(MockFixedSizedNetwork, [1, 2, 3])
 
     def test_trajectory_too_short(self):
         """
@@ -34,10 +34,10 @@ class TestSynchronous(unittest.TestCase):
         than 1
         """
         with self.assertRaises(ValueError):
-            list(trajectory(MockFixedSizedNetwork(), [1, 2, 3], timesteps=0))
+            trajectory(MockFixedSizedNetwork(), [1, 2, 3], timesteps=0)
 
         with self.assertRaises(ValueError):
-            list(trajectory(MockFixedSizedNetwork(), [1, 2, 3], timesteps=-1))
+            trajectory(MockFixedSizedNetwork(), [1, 2, 3], timesteps=-1)
 
     def test_trajectory_eca(self):
         """
@@ -45,14 +45,14 @@ class TestSynchronous(unittest.TestCase):
         """
         rule30 = ECA(30)
         with self.assertRaises(ValueError):
-            list(trajectory(rule30, []))
+            trajectory(rule30, [])
 
         xs = [0, 1, 0]
-        got = list(trajectory(rule30, xs))
+        got = trajectory(rule30, xs)
         self.assertEqual([0, 1, 0], xs)
         self.assertEqual([[0, 1, 0], [1, 1, 1]], got)
 
-        got = list(trajectory(rule30, xs, timesteps=2))
+        got = trajectory(rule30, xs, timesteps=2)
         self.assertEqual([0, 1, 0], xs)
         self.assertEqual([[0, 1, 0], [1, 1, 1], [0, 0, 0]], got)
 
@@ -62,14 +62,14 @@ class TestSynchronous(unittest.TestCase):
         """
         rule30 = ECA(30)
         with self.assertRaises(ValueError):
-            list(trajectory(rule30, [], encode=True))
+            trajectory(rule30, [], encode=True)
 
         state = [0, 1, 0]
-        got = list(trajectory(rule30, state, encode=True))
+        got = trajectory(rule30, state, encode=True)
         self.assertEqual([0, 1, 0], state)
         self.assertEqual([2, 7], got)
 
-        got = list(trajectory(rule30, state, timesteps=2, encode=True))
+        got = trajectory(rule30, state, timesteps=2, encode=True)
         self.assertEqual([0, 1, 0], state)
         self.assertEqual([2, 7, 0], got)
 
@@ -84,11 +84,11 @@ class TestSynchronous(unittest.TestCase):
         )
 
         state = [0, 0]
-        got = list(trajectory(net, state))
+        got = trajectory(net, state)
         self.assertEqual([0, 0], state)
         self.assertEqual([[0, 0], [0, 1]], got)
 
-        got = list(trajectory(net, state, timesteps=3))
+        got = trajectory(net, state, timesteps=3)
         self.assertEqual([0, 0], state)
         self.assertEqual([[0, 0], [0, 1], [0, 1], [0, 1]], got)
 
@@ -103,11 +103,11 @@ class TestSynchronous(unittest.TestCase):
         )
 
         state = [0, 0]
-        got = list(trajectory(net, state, encode=True))
+        got = trajectory(net, state, encode=True)
         self.assertEqual([0, 0], state)
         self.assertEqual([0, 2], got)
 
-        got = list(trajectory(net, state, timesteps=3, encode=True))
+        got = trajectory(net, state, timesteps=3, encode=True)
         self.assertEqual([0, 0], state)
         self.assertEqual([0, 2, 2, 2], got)
 
@@ -119,7 +119,7 @@ class TestSynchronous(unittest.TestCase):
                             ((0, 2), {'01', '10', '11'}),
                             ((0, 1), {'11'})])
         state = [0, 1, 0]
-        got = list(trajectory(net, state, 3))
+        got = trajectory(net, state, 3)
         self.assertEqual([[0, 1, 0], [1, 0, 0], [0, 1, 0], [1, 0, 0]], got)
         self.assertEqual([0, 1, 0], state)
 
@@ -131,7 +131,7 @@ class TestSynchronous(unittest.TestCase):
         ``transitions`` should raise a type error if ``net`` is not a network
         """
         with self.assertRaises(TypeError):
-            list(transitions(MockObject(), 5))
+            transitions(MockObject(), 5)
 
     def test_transitions_not_fixed_sized(self):
         """
@@ -139,7 +139,7 @@ class TestSynchronous(unittest.TestCase):
         and ``size`` is ``None``
         """
         with self.assertRaises(ValueError):
-            list(transitions(ECA(30), size=None))
+            transitions(ECA(30), size=None)
 
     def test_transitions_fixed_sized(self):
         """
@@ -147,7 +147,7 @@ class TestSynchronous(unittest.TestCase):
         ``size`` is not ``None``
         """
         with self.assertRaises(ValueError):
-            list(transitions(MockFixedSizedNetwork, size=3))
+            transitions(MockFixedSizedNetwork, size=3)
 
     def test_transitions_eca(self):
         """
@@ -155,13 +155,13 @@ class TestSynchronous(unittest.TestCase):
         """
         rule30 = ECA(30)
 
-        got = list(transitions(rule30, size=1))
+        got = transitions(rule30, size=1)
         self.assertEqual([[0], [0]], got)
 
-        got = list(transitions(rule30, size=2))
+        got = transitions(rule30, size=2)
         self.assertEqual([[0, 0], [1, 0], [0, 1], [0, 0]], got)
 
-        got = list(transitions(rule30, size=3))
+        got = transitions(rule30, size=3)
         self.assertEqual([[0, 0, 0], [1, 1, 1], [1, 1, 1], [1, 0, 0],
                           [1, 1, 1], [0, 0, 1], [0, 1, 0], [0, 0, 0]], got)
 
@@ -171,13 +171,13 @@ class TestSynchronous(unittest.TestCase):
         """
         rule30 = ECA(30)
 
-        got = list(transitions(rule30, size=1, encode=True))
+        got = transitions(rule30, size=1, encode=True)
         self.assertEqual([0, 0], got)
 
-        got = list(transitions(rule30, size=2, encode=True))
+        got = transitions(rule30, size=2, encode=True)
         self.assertEqual([0, 1, 2, 0], got)
 
-        got = list(transitions(rule30, size=3, encode=True))
+        got = transitions(rule30, size=3, encode=True)
         self.assertEqual([0, 7, 7, 1, 7, 4, 2, 0], got)
 
     def test_transitions_wtnetwork(self):
@@ -190,7 +190,7 @@ class TestSynchronous(unittest.TestCase):
             theta=WTNetwork.positive_threshold
         )
 
-        got = list(transitions(net))
+        got = transitions(net)
         self.assertEqual([[0, 1], [1, 0], [0, 1], [1, 1]], got)
 
     def test_transitions_wtnetwork_encoded(self):
@@ -203,7 +203,7 @@ class TestSynchronous(unittest.TestCase):
             theta=WTNetwork.positive_threshold
         )
 
-        got = list(transitions(net, encode=True))
+        got = transitions(net, encode=True)
         self.assertEqual([2, 1, 2, 3], got)
 
     def test_transitions_logicnetwork(self):
@@ -211,7 +211,7 @@ class TestSynchronous(unittest.TestCase):
         test `transitions` on `LogicNetwork`s
         """
         net = LogicNetwork([((1,), {'0', '1'}), ((0,), {'1'})])
-        got = list(transitions(net))
+        got = transitions(net)
         self.assertEqual([[1, 0], [1, 1], [1, 0], [1, 1]], got)
 
     def test_transitions_logicnetwork_encoded(self):
@@ -219,7 +219,7 @@ class TestSynchronous(unittest.TestCase):
         test `transitions` on `LogicNetwork`s, states encoded
         """
         net = LogicNetwork([((1,), {'0', '1'}), ((0,), {'1'})])
-        got = list(transitions(net, encode=True))
+        got = transitions(net, encode=True)
         self.assertEqual([1, 3, 1, 3], got)
 
     def test_transition_graph_not_network(self):
@@ -268,13 +268,13 @@ class TestSynchronous(unittest.TestCase):
         nor a networkx digraph
         """
         with self.assertRaises(TypeError):
-            list(attractors('blah'))
+            attractors('blah')
 
         with self.assertRaises(TypeError):
-            list(attractors(MockObject()))
+            attractors(MockObject())
 
         with self.assertRaises(TypeError):
-            list(attractors(nx.Graph()))
+            attractors(nx.Graph())
 
     def test_attractors_variable_sized(self):
         """
@@ -282,7 +282,7 @@ class TestSynchronous(unittest.TestCase):
         network and ``size`` is ``None``
         """
         with self.assertRaises(ValueError):
-            list(attractors(ECA(30), size=None))
+            attractors(ECA(30), size=None)
 
     def test_attractors_fixed_sized(self):
         """
@@ -290,10 +290,10 @@ class TestSynchronous(unittest.TestCase):
         network or a networkx digraph, and ``size`` is not ``None``
         """
         with self.assertRaises(ValueError):
-            list(attractors(MockFixedSizedNetwork(), size=5))
+            attractors(MockFixedSizedNetwork(), size=5)
 
         #  with self.assertRaises(ValueError):
-        #      list(attractors(nx.DiGraph(), size=5))
+        #      attractors(nx.DiGraph(), size=5)
 
     def test_attractors_eca(self):
         """
@@ -304,7 +304,7 @@ class TestSynchronous(unittest.TestCase):
                     (ECA(110), 3, 1), (ECA(110), 4, 3), (ECA(110), 5, 1),
                     (ECA(110), 6, 3)]
         for rule, width, size in networks:
-            self.assertEqual(size, len(list(attractors(rule, width))))
+            self.assertEqual(size, len(attractors(rule, width)))
 
     def test_attractors_wtnetworks(self):
         """
@@ -312,14 +312,14 @@ class TestSynchronous(unittest.TestCase):
         """
         networks = [(s_pombe, 13), (s_cerevisiae, 7), (c_elegans, 5)]
         for net, size in networks:
-            self.assertEqual(size, len(list(attractors(net))))
+            self.assertEqual(size, len(attractors(net)))
 
     def test_attractors_transition_graph(self):
         """
         test ``attractors`` on ``s_pombe`` transition graph
         """
-        att_from_graph = list(attractors(transition_graph(s_pombe)))
-        att_from_network = list(attractors(s_pombe))
+        att_from_graph = attractors(transition_graph(s_pombe))
+        att_from_network = attractors(s_pombe)
 
         for (a, b) in zip(att_from_graph, att_from_network):
             a.sort()