this fixes #292

mode model graphs to `GraphMachien.model_graphs`; add long_description in markdown to setup.py; remove python 3.3 (EOL) from travis; add test_pickle_model to core; move DummyModel/TestContext to utils; let TestRep inherit from TestCore; adapt test to not directly use model.graph;

Author: aleneum

.travis.yml

@@ -3,7 +3,6 @@ python:
   - "3.6"
   - "3.5"
   - "3.4"
-  - "3.3"
   - "2.7"
 install:
   - pip install python-coveralls coverage

Changelog.md

@@ -3,6 +3,7 @@
 ## 0.6.5 ()
 
 - Feature #287: Embedding `HierarchicalMachine` will now reuse the machine's `initial` state. Passing `initial: False` overrides this(thanks @mrjogo).
+- Bug #292: Models using `GraphMashine` were not picklable in the past due to `graph` property. Graphs for each model are now stored in `GraphMachine.model_graphs` (thanks @ansumanm)
 
 ## 0.6.4 (January, 2018)
 

setup.py

@@ -4,6 +4,9 @@
 with open('transitions/version.py') as f:
     exec(f.read())
 
+with open('README.md') as file:
+    long_description = file.read()
+
 if len(set(('test', 'easy_install')).intersection(sys.argv)) > 0:
     import setuptools
 
@@ -22,6 +25,7 @@
     name="transitions",
     version=__version__,
     description="A lightweight, object-oriented Python state machine implementation.",
+    long_description=long_description,
     maintainer='Tal Yarkoni',
     maintainer_email='[email protected]',
     url='http://github.com/pytransitions/transitions',

test.py

@@ -485,6 +485,20 @@ def test_pickle(self):
         self.assertEqual(m.state, m2.state)
         m2.run()
 
+    def test_pickle_model(self):
+        import sys
+        if sys.version_info < (3, 4):
+            import dill as pickle
+        else:
+            import pickle
+
+        self.stuff.to_B()
+        dump = pickle.dumps(self.stuff)
+        self.assertIsNotNone(dump)
+        model2 = pickle.loads(dump)
+        self.assertEqual(self.stuff.state, model2.state)
+        model2.to_F()
+
     def test_queued(self):
         states = ['A', 'B', 'C', 'D']
         # Define with list of dictionaries

tests/test_core.py

@@ -4,11 +4,12 @@
     pass
 
 from .utils import Stuff
+from .test_core import TestTransitions
 
 from transitions.extensions import MachineFactory
 from transitions.extensions.diagrams import rep
 from transitions.extensions.nesting import NestedState
-from unittest import TestCase, skipIf
+from unittest import skipIf
 from functools import partial
 import tempfile
 import os
@@ -24,7 +25,7 @@ def edge_label_from_transition_label(label):
     return label.split(' | ')[0].split(' [')[0]  # if no condition, label is returned; returns first event only
 
 
-class TestRep(TestCase):
+class TestRep(TestTransitions):
 
     def test_rep_string(self):
         self.assertEqual(rep("string"), "string")
@@ -80,11 +81,12 @@ def __repr__(self):
 
 
 @skipIf(pgv is None, 'Graph diagram requires pygraphviz')
-class TestDiagrams(TestCase):
+class TestDiagrams(TestTransitions):
 
-    def setUp(self):
-        self.machine_cls = MachineFactory.get_predefined(graph=True)
+    machine_cls = MachineFactory.get_predefined(graph=True)
 
+    def setUp(self):
+        self.stuff = Stuff(machine_cls=self.machine_cls)
         self.states = ['A', 'B', 'C', 'D']
         self.transitions = [
             {'trigger': 'walk', 'source': 'A', 'dest': 'B'},
@@ -154,12 +156,12 @@ def test_multi_model_state(self):
         m2 = Stuff(machine_cls=None)
         m = self.machine_cls(model=[m1, m2], states=self.states, transitions=self.transitions, initial='A')
         m1.walk()
-        self.assertEqual(m1.graph.get_node(m1.state).attr['color'],
-                         m1.graph.style_attributes['node']['active']['color'])
-        self.assertEqual(m2.graph.get_node(m1.state).attr['color'],
-                         m2.graph.style_attributes['node']['default']['color'])
+        self.assertEqual(m1.get_graph().get_node(m1.state).attr['color'],
+                         m1.get_graph().style_attributes['node']['active']['color'])
+        self.assertEqual(m2.get_graph().get_node(m1.state).attr['color'],
+                         m2.get_graph().style_attributes['node']['default']['color'])
         # backwards compatibility test
-        self.assertEqual(m.get_graph(), m1.get_graph())
+        self.assertEqual(id(m.get_graph()), id(m1.get_graph()))
 
     def test_model_method_collision(self):
         class GraphModel:
@@ -202,16 +204,16 @@ def test_roi(self):
 @skipIf(pgv is None, 'Graph diagram requires pygraphviz')
 class TestDiagramsLocked(TestDiagrams):
 
-    def setUp(self):
-        super(TestDiagramsLocked, self).setUp()
-        self.machine_cls = MachineFactory.get_predefined(graph=True, locked=True)
+    machine_cls = MachineFactory.get_predefined(graph=True, locked=True)
 
 
 @skipIf(pgv is None, 'NestedGraph diagram requires pygraphviz')
 class TestDiagramsNested(TestDiagrams):
 
+    machine_cls = MachineFactory.get_predefined(graph=True, nested=True)
+
     def setUp(self):
-        self.machine_cls = MachineFactory.get_predefined(graph=True, nested=True)
+        super(TestDiagramsNested, self).setUp()
         self.states = ['A', 'B',
                        {'name': 'C', 'children': [{'name': '1', 'children': ['a', 'b', 'c']},
                                                   '2', '3']}, 'D']

tests/test_graphing.py

@@ -11,7 +11,7 @@
 from transitions.extensions.nesting import NestedState
 from .test_nesting import TestTransitions as TestsNested
 from .test_core import TestTransitions as TestCore
-from .utils import Stuff
+from .utils import Stuff, DummyModel, TestContext
 
 try:
     from unittest.mock import MagicMock
@@ -166,28 +166,15 @@ def __exit__(self, *exc):
 
 class TestMultipleContexts(TestCore):
 
-    class DummyModel(object):
-        pass
-
-    class TestContext(object):
-        def __init__(self, event_list):
-            self._event_list = event_list
-
-        def __enter__(self):
-            self._event_list.append((self, "enter"))
-
-        def __exit__(self, type, value, traceback):
-            self._event_list.append((self, "exit"))
-
     def setUp(self):
         self.event_list = []
 
-        self.s1 = self.DummyModel()
+        self.s1 = DummyModel()
 
-        self.c1 = self.TestContext(event_list=self.event_list)
-        self.c2 = self.TestContext(event_list=self.event_list)
-        self.c3 = self.TestContext(event_list=self.event_list)
-        self.c4 = self.TestContext(event_list=self.event_list)
+        self.c1 = TestContext(event_list=self.event_list)
+        self.c2 = TestContext(event_list=self.event_list)
+        self.c3 = TestContext(event_list=self.event_list)
+        self.c4 = TestContext(event_list=self.event_list)
 
         self.stuff = Stuff(machine_cls=MachineFactory.get_predefined(locked=True), extra_kwargs={
             'machine_context': [self.c1, self.c2]

tests/test_threading.py

@@ -90,3 +90,18 @@ def __init__(self, states, initial='A'):
     @staticmethod
     def this_passes():
         return True
+
+
+class DummyModel(object):
+    pass
+
+
+class TestContext(object):
+    def __init__(self, event_list):
+        self._event_list = event_list
+
+    def __enter__(self):
+        self._event_list.append((self, "enter"))
+
+    def __exit__(self, type, value, traceback):
+        self._event_list.append((self, "exit"))

tests/utils.py

@@ -1,5 +1,5 @@
 [tox]
-envlist = py2, py3, py33, py34, py35, codestyle, check-manifest
+envlist = py2, py3, py34, py35, py36, codestyle, check-manifest
 skip_missing_interpreters = True
 
 [testenv]

tox.ini

@@ -252,8 +252,8 @@ def execute(self, event_data):
 
         for cond in self.conditions:
             if not cond.check(event_data):
-                _LOGGER.debug("%sTransition condition failed: %s() does not " +
-                              "return %s. Transition halted.", event_data.machine.name, cond.func, cond.target)
+                _LOGGER.debug("%sTransition condition failed: %s() does not return %s. Transition halted.",
+                              event_data.machine.name, cond.func, cond.target)
                 return False
         for func in itertools.chain(machine.before_state_change, self.before):
             machine.callback(func, event_data)

transitions/core.py

@@ -274,16 +274,16 @@ def _change_state(self, event_data):
         machine = event_data.machine
         model = event_data.model
         dest = machine.get_state(self.dest)
+        graph = model.get_graph()
 
         # Mark the active node
-        machine.reset_graph_style(model.graph)
+        machine.reset_graph_style(graph)
 
         # Mark the previous node and path used
         if self.source is not None:
             source = machine.get_state(self.source)
-            machine.set_node_state(model.graph, source.name,
-                                   state='previous')
-            machine.set_node_state(model.graph, dest.name, state='active')
+            machine.set_node_state(graph, source.name, state='previous')
+            machine.set_node_state(graph, dest.name, state='active')
 
             if getattr(source, 'children', []):
                 source = source.name + '_anchor'
@@ -293,8 +293,7 @@ def _change_state(self, event_data):
                 dest = dest.name + '_anchor'
             else:
                 dest = dest.name
-            machine.set_edge_state(model.graph, source, dest,
-                                   state='previous', label=event_data.event.name)
+            machine.set_edge_state(graph, source, dest, state='previous', label=event_data.event.name)
 
         _super(TransitionGraphSupport, self)._change_state(event_data)  # pylint: disable=protected-access
 
@@ -307,23 +306,29 @@ class GraphMachine(Machine):
             transition_cls (cls): TransitionGraphSupport
     """
 
-    _pickle_blacklist = ['graph']
+    _pickle_blacklist = ['model_graphs']
     transition_cls = TransitionGraphSupport
 
+    # model_graphs cannot be pickled. Omit them.
     def __getstate__(self):
         return {k: v for k, v in self.__dict__.items() if k not in self._pickle_blacklist}
 
     def __setstate__(self, state):
         self.__dict__.update(state)
+        self.model_graphs = {}  # reinitialize new model_graphs
         for model in self.models:
-            graph = self._get_graph(model, title=self.title)
-            self.set_node_style(graph, model.state, 'active')
+            try:
+                graph = self._get_graph(model, title=self.title)
+                self.set_node_style(graph, model.state, 'active')
+            except AttributeError:
+                _LOGGER.warning("Graph for model could not be initialized after pickling.")
 
     def __init__(self, *args, **kwargs):
         # remove graph config from keywords
         self.title = kwargs.pop('title', 'State Machine')
         self.show_conditions = kwargs.pop('show_conditions', False)
         self.show_auto_transitions = kwargs.pop('show_auto_transitions', False)
+        self.model_graphs = {}
 
         # Update March 2017: This temporal overwrite does not work
         # well with inheritance. Since the tests pass I will disable it
@@ -346,7 +351,7 @@ def __init__(self, *args, **kwargs):
                 raise AttributeError('Model already has a get_graph attribute. Graph retrieval cannot be bound.')
             setattr(model, 'get_graph', partial(self._get_graph, model))
             model.get_graph()
-            self.set_node_state(model.graph, self.initial, 'active')
+            self.set_node_state(self.model_graphs[model], self.initial, 'active')
 
         # for backwards compatibility assign get_combined_graph to get_graph
         # if model is not the machine
@@ -356,12 +361,12 @@ def __init__(self, *args, **kwargs):
     def _get_graph(self, model, title=None, force_new=False, show_roi=False):
         if title is None:
             title = self.title
-        if not hasattr(model, 'graph') or force_new:
-            model.graph = NestedGraph(self).get_graph(title) if isinstance(self, HierarchicalMachine) \
+        if model not in self.model_graphs or force_new:
+            self.model_graphs[model] = NestedGraph(self).get_graph(title) if isinstance(self, HierarchicalMachine) \
                 else Graph(self).get_graph(title)
-            self.set_node_state(model.graph, model.state, state='active')
+            self.set_node_state(self.model_graphs[model], model.state, state='active')
 
-        return model.graph if not show_roi else self._graph_roi(model)
+        return self.model_graphs[model] if not show_roi else self._graph_roi(model)
 
     def get_combined_graph(self, title=None, force_new=False, show_roi=False):
         """ This method is currently equivalent to 'get_graph' of the first machine's model.
@@ -374,7 +379,7 @@ def get_combined_graph(self, title=None, force_new=False, show_roi=False):
                 the current state.
         Returns: AGraph of the first machine's model.
         """
-        _LOGGER.info('Returning graph of the first model. In future releases, this ' +
+        _LOGGER.info('Returning graph of the first model. In future releases, this '
                      'method will return a combined graph of all models.')
         return self._get_graph(self.models[0], title, force_new, show_roi)
 
@@ -439,9 +444,8 @@ def set_node_state(self, graph, node_name, state='default'):
             func = self.set_graph_style
         func(graph, node, state)
 
-    @staticmethod
-    def _graph_roi(model):
-        graph = model.graph
+    def _graph_roi(self, model):
+        graph = model.get_graph()
         filtered = graph.copy()
 
         kept_nodes = set()