From 0aca75db84fadf7e5f13c0d60c44b666335f8308 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 1 Apr 2021 11:34:19 +0200
Subject: [PATCH 01/28] introducing Plan

---
 CHANGELOG.rst                             |   1 +
 src/compas_fab/datastructures/__init__.py |  14 +++
 src/compas_fab/datastructures/plan.py     | 131 ++++++++++++++++++++++
 3 files changed, 146 insertions(+)
 create mode 100644 src/compas_fab/datastructures/__init__.py
 create mode 100644 src/compas_fab/datastructures/plan.py

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index f795cd1bd..1fbe1dc0a 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -13,6 +13,7 @@ Unreleased
 **Added**
 
 * Added support for MoveIt on ROS Noetic
+* In`compas.datastructures`, added `Plan`, `PlannedAction`, `Action` and `IntegerIdGenerator`
 
 **Changed**
 
diff --git a/src/compas_fab/datastructures/__init__.py b/src/compas_fab/datastructures/__init__.py
new file mode 100644
index 000000000..0d45966f7
--- /dev/null
+++ b/src/compas_fab/datastructures/__init__.py
@@ -0,0 +1,14 @@
+from .plan import (
+    Action,
+    IntegerIdGenerator,
+    Plan,
+    PlannedAction
+)
+
+
+__all__ = [
+    'Action',
+    'IntegerIdGenerator',
+    'Plan',
+    'PlannedAction',
+]
diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
new file mode 100644
index 000000000..f090898ec
--- /dev/null
+++ b/src/compas_fab/datastructures/plan.py
@@ -0,0 +1,131 @@
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+from collections import OrderedDict
+
+from compas.utilities import DataEncoder
+from compas.utilities import DataDecoder
+
+
+__all__ = [
+    'Action',
+    'IntegerIdGenerator',
+    'Plan',
+    'PlannedAction',
+]
+
+
+class IntegerIdGenerator(object):
+    def __init__(self, start_value=1):
+        self._generator = count(start_value)
+
+    def __next__(self):
+        yield next(self._generator)
+
+
+class Plan(object):
+    def __init__(self, planned_actions=None, id_generator=None):
+        self.planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
+        if id_generator is None:
+            start_value = max(self.planned_actions.keys()) + 1 if self.planned_actions else 1
+            id_generator = IntegerIdGenerator(start_value)
+        self._id_generator = id_generator
+
+    def plan_action(self, action, dependency_ids, namespace=None):
+        action_id = self._get_next_action_id()
+        planned_action = PlannedAction(action_id, action, dependency_ids, namespace)
+        self.planned_actions[action_id] = planned_action
+        return action_id
+
+    def append_action(self, action, namespace=None):
+        dependency_ids = set()
+        if self.planned_actions:
+            last_action_id = self._get_last_action_id()
+            dependency_ids = {last_action_id}
+        action_id = self.plan_action(action, dependency_ids, namespace)
+        planned_action = self.planned_actions[action_id]
+        return action_id
+
+    def remove_action_by_id(self, action_id):
+        planned_action = self.planned_actions.pop(action_id)
+        for pa in self.planned_actions.values():
+            pa.dependency_ids.discard(action_id)
+        return planned_action.action
+
+    def _get_last_action_id(self):
+        last_action_id, last_action = self.planned_actions.popitem()
+        self.planned_actions[last_action_id] = last_action
+        return last_action_id
+
+    def _get_next_action_id(self):
+        return next(self._id_generator)
+
+    def get_namespaces(self):
+        return set(a.namespace for a in self.planned_actions.values())
+
+    @property
+    def data(self):
+        return dict(
+            planned_actions=[p.to_data() for p in self.planned_actions.values()]
+        )
+
+    @classmethod
+    def from_data(cls, data):
+        return cls([PlannedAction.from_data(d) for d in data['planned_actions']])
+
+    def to_data(self):
+        return self.data
+
+
+class PlannedAction(object):
+    def __init__(self, action_id, action, dependency_ids, namespace=None):
+        self.id = action_id
+        self.action = action
+        self.dependency_ids = set(dependency_ids)
+        self.namespace = namespace
+
+    def __str__(self):
+        return 'PlannedAction<id={}, action={}, namespace={}>'.format(self.id, self.action, self.namespace)
+
+    @property
+    def data(self):
+        return dict(
+            id=self.id,
+            action=self.action.to_data(),
+            dependency_ids=self.dependency_ids,
+            namespace=self.namespace,
+        )
+
+    @classmethod
+    def from_data(cls, data):
+        return cls(data['id'], Action.from_data(data['action']), data['dependency_ids'], data['namespace'])
+
+    def to_data(self):
+        return self.data
+
+
+class Action(object):
+    def __init__(self, name, parameters=None):
+        self.name = name
+        self.parameters = parameters or {}
+
+    def __str__(self):
+        return 'Action<name={}>'.format(self.name)
+
+    @property
+    def data(self):
+        return dict(
+            name=self.name,
+            parameters={param_key: DataEncoder().default(p) if hasattr(p, 'to_data') else p for param_key, p in
+                        self.parameters.items()},
+        )
+
+    @classmethod
+    def from_data(cls, data):
+        return cls(data['name'],
+                   {param_key: DataDecoder().object_hook(p) if hasattr(p, '__iter__') else p for param_key, p in
+                    data['parameters'].items()})
+
+    def to_data(self):
+        return self.data

From da4fe74472c2a0fa2f59ae308b7f532f0491cc9f Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 1 Apr 2021 11:43:31 +0200
Subject: [PATCH 02/28] lint

---
 src/compas_fab/datastructures/plan.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index f090898ec..fb474f2b4 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -3,6 +3,7 @@
 from __future__ import print_function
 
 from collections import OrderedDict
+from itertools import count
 
 from compas.utilities import DataEncoder
 from compas.utilities import DataDecoder
@@ -43,9 +44,7 @@ def append_action(self, action, namespace=None):
         if self.planned_actions:
             last_action_id = self._get_last_action_id()
             dependency_ids = {last_action_id}
-        action_id = self.plan_action(action, dependency_ids, namespace)
-        planned_action = self.planned_actions[action_id]
-        return action_id
+        return self.plan_action(action, dependency_ids, namespace)
 
     def remove_action_by_id(self, action_id):
         planned_action = self.planned_actions.pop(action_id)

From b7d8835aabd2162e0af70f33a7f6ed7bf3d6dc85 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Tue, 6 Apr 2021 12:41:03 +0200
Subject: [PATCH 03/28] docstrings, inherit from compas.base.Base, safety
 checks

---
 src/compas_fab/datastructures/plan.py | 248 +++++++++++++++++++++-----
 1 file changed, 205 insertions(+), 43 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index fb474f2b4..4b2869ec9 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -2,11 +2,11 @@
 from __future__ import division
 from __future__ import print_function
 
+import threading
 from collections import OrderedDict
 from itertools import count
 
-from compas.utilities import DataEncoder
-from compas.utilities import DataDecoder
+from compas.base import Base
 
 
 __all__ = [
@@ -18,35 +18,119 @@
 
 
 class IntegerIdGenerator(object):
+    """Generator object yielding integers sequentially in a thread safe manner.
+
+    Parameters
+    ----------
+    start_value : :obj:`int`
+        First value to be yielded by the generator.
+    """
     def __init__(self, start_value=1):
+        self._lock = threading.Lock()
         self._generator = count(start_value)
 
     def __next__(self):
-        yield next(self._generator)
+        with self._lock:
+            yield next(self._generator)
+
+
+class DependencyIdException(Exception):
+    """Indicates invalid ids given as dependencies."""
+    def __init__(self, invalid_ids, pa_id=None):
+        message = self.compose_message(invalid_ids, pa_id)
+        super(DependencyIdException, self).__init__(message)
+
+    @staticmethod
+    def compose_message(invalid_ids, pa_id):
+        if pa_id:
+            return 'Planned action {} has invalid dependency ids {}'.format(pa_id, invalid_ids)
+        return 'Found invalid dependency ids {}'.format(invalid_ids)
 
 
-class Plan(object):
+class Plan(Base):
+    """Data structure for holding the information of a partially ordered plan
+    (a directed acyclic graph).  The content of any event of the plan is contained
+    in an :class:`compas_fab.datastructures.Action`. An event is scheduled and
+    added to the plan through a :class:`compas_fab.datastructures.PlannedAction`.
+    The dependency ids of a planned action can be thought of as pointers to the
+    parents of that planned action.
+
+    Parameters
+    ----------
+    planned_actions : list of :class:`compas_fab.datastructures.PlannedAction`
+        The planned actions will be stored as an ordered dictionary, so their
+        ids should be distinct.  Defaults to an empty list.
+    id_generator : Generator[Hashable, None, None]
+        Object which generates keys (via ``next()``) for
+        :class:`compas_fab.datastructures.Action`s added using this object's
+        methods.  Defaults to :class:`compas_fab.datastructures.IntegerIdGenerator`.
+    """
     def __init__(self, planned_actions=None, id_generator=None):
+        planned_actions = planned_actions or []
         self.planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
         if id_generator is None:
-            start_value = max(self.planned_actions.keys()) + 1 if self.planned_actions else 1
+            try:
+                start_value = max(self.planned_actions.keys()) + 1 if self.planned_actions else 1
+            except Exception:
+                raise Exception('Given ids not compatiable with default id_generator.')
             id_generator = IntegerIdGenerator(start_value)
         self._id_generator = id_generator
 
-    def plan_action(self, action, dependency_ids, namespace=None):
+    def plan_action(self, action, dependency_ids):
+        """Adds the action to the plan with the given dependencies,
+        and generates an id for the newly planned action.
+
+        Parameters
+        ----------
+        action : :class:`comaps_fab.datastructures.Action`
+            The action to be added to the plan.
+        dependency_ids : set or list
+            The keys of the already planned actions that the new action
+            is dependent on.
+
+        Returns
+        -------
+            The id of the newly planned.
+        """
+        self.check_dependency_ids(dependency_ids)
         action_id = self._get_next_action_id()
-        planned_action = PlannedAction(action_id, action, dependency_ids, namespace)
+        planned_action = PlannedAction(action_id, action, dependency_ids)
         self.planned_actions[action_id] = planned_action
         return action_id
 
-    def append_action(self, action, namespace=None):
+    def append_action(self, action):
+        """Adds the action to the plan dependent on the last action added
+        to the plan, and generates an id for this newly planned action.
+
+        Parameters
+        ----------
+        action : :class:`comaps_fab.datastructures.Action`
+            The action to be added to the plan.
+
+        Returns
+        -------
+            The id of the newly planned.
+        """
         dependency_ids = set()
         if self.planned_actions:
             last_action_id = self._get_last_action_id()
             dependency_ids = {last_action_id}
-        return self.plan_action(action, dependency_ids, namespace)
+        return self.plan_action(action, dependency_ids)
 
     def remove_action_by_id(self, action_id):
+        """Removes the action with the given id from the plan and all its
+        dependencies.
+
+        Parameters
+        ----------
+        action_id : Hashable
+            Id of the planned action to be removed.
+
+        Returns
+        -------
+        :class:`compas_fab.datastructure.Action`
+        The action of the removed :class:`compas_fab.datastructure.PlannedAction`
+        """
         planned_action = self.planned_actions.pop(action_id)
         for pa in self.planned_actions.values():
             pa.dependency_ids.discard(action_id)
@@ -60,51 +144,139 @@ def _get_last_action_id(self):
     def _get_next_action_id(self):
         return next(self._id_generator)
 
-    def get_namespaces(self):
-        return set(a.namespace for a in self.planned_actions.values())
+    def check_dependency_ids(self, dependency_ids, planned_action_id=None):
+        """Checks whether the given dependency ids exist in the plan.
+
+        Parameters
+        ----------
+        dependency_ids : set or list
+            The dependency ids to be validated.
+        planned_action_id : Hashable
+            The id of the associated planned action.  Used only in
+            the error message.  Defaults to ``None``.
+
+        Raises
+        ------
+        :class:`compas_fab.datastructures.DependencyIdException`
+        """
+        dependency_ids = set(dependency_ids)
+        if not dependency_ids.issubset(self.planned_actions):
+            invalid_ids = dependency_ids.difference(self.planned_actions)
+            raise DependencyIdException(invalid_ids, planned_action_id)
+
+    def check_all_dependency_ids(self):
+        """Checks whether the dependency ids of all the planned actions
+        are ids of planned actions in the plan.
+
+        Raises
+        ------
+        :class:`compas_fab.datastructures.DependencyIdException`
+        """
+        for pa_id, planned_action in self.planned_actions.items():
+            self.check_dependency_ids(planned_action.dependency_ids, pa_id)
+
+    def check_for_cycles(self):
+        """"Checks whether cycles exist in the dependency graph."""
+        self.check_all_dependency_ids()
+
+        def helper(cur, v, r):
+            v[cur] = True
+            r[cur] = True
+            for dep_id in self.planned_actions[cur].dependency_ids:
+                if not v[dep_id]:
+                    helper(dep_id, v, r)
+                elif r[dep_id]:
+                    raise Exception("Cycle found with action ids {}".format([pa_id for pa_id, seen in r.items() if seen]))
+            r[cur] = False
+
+        visited = {pa_id: False for pa_id in self.planned_actions}
+        rec_dict = {pa_id: False for pa_id in self.planned_actions}
+
+        for pa_id in self.planned_actions:
+            if not visited[pa_id]:
+                helper(pa_id, visited, rec_dict)
+
+    def linear_sort(self):
+        """Sorts the planned actions linearly respecting the dependency ids.
+
+        Returns
+        -------
+        :obj:`list` of :class:`compas_fab.datastructure.Action`
+        """
+        self.check_for_cycles()
+
+        def helper(s, v, cur_id):
+            v.add(cur_id)
+            action = self.planned_actions[cur_id]
+            for dep_id in action.dependency_ids:
+                if dep_id not in v:
+                    helper(s, v, dep_id)
+            s.append(action)
+
+        stack = []
+        visited = set()
+
+        for action_id in self.planned_actions:
+            if action_id not in visited:
+                helper(stack, visited, action_id)
+
+        return stack
 
     @property
     def data(self):
         return dict(
-            planned_actions=[p.to_data() for p in self.planned_actions.values()]
+            planned_actions=list(self.planned_actions.values())
         )
 
-    @classmethod
-    def from_data(cls, data):
-        return cls([PlannedAction.from_data(d) for d in data['planned_actions']])
-
-    def to_data(self):
-        return self.data
 
+class PlannedAction(Base):
+    """Represents an action which has been scheduled in a plan.
 
-class PlannedAction(object):
-    def __init__(self, action_id, action, dependency_ids, namespace=None):
+    Parameters
+    ----------
+    action_id : Hashable
+        An identifier of the action.  Used by other actions and the plan
+        it is associated with.
+    action : :class:`compas_fab.datastructures.Action`
+        The action to be planned.
+    dependency_ids : set or list
+        The ids of the actions upon which `action` is dependent.
+    """
+    def __init__(self, action_id, action, dependency_ids):
         self.id = action_id
         self.action = action
-        self.dependency_ids = set(dependency_ids)
-        self.namespace = namespace
+        self.dependency_ids = dependency_ids
+
+    @property
+    def dependency_ids(self):
+        return self._dependency_ids
+
+    @dependency_ids.setter
+    def dependency_ids(self, value):
+        self._dependency_ids = set(value)
 
     def __str__(self):
-        return 'PlannedAction<id={}, action={}, namespace={}>'.format(self.id, self.action, self.namespace)
+        return 'PlannedAction<id={}, action={}>'.format(self.id, self.action)
 
     @property
     def data(self):
         return dict(
             id=self.id,
-            action=self.action.to_data(),
+            action=self.action,
             dependency_ids=self.dependency_ids,
-            namespace=self.namespace,
         )
 
-    @classmethod
-    def from_data(cls, data):
-        return cls(data['id'], Action.from_data(data['action']), data['dependency_ids'], data['namespace'])
-
-    def to_data(self):
-        return self.data
 
+class Action(Base):
+    """Abstract representation of an event independent of its timing.
 
-class Action(object):
+    Parameters
+    ----------
+    name : :obj:`str`
+        The name of the action.
+    parameters : dict
+        Any other content associated to the action housed in key-value pairs.
+    """
     def __init__(self, name, parameters=None):
         self.name = name
         self.parameters = parameters or {}
@@ -116,15 +288,5 @@ def __str__(self):
     def data(self):
         return dict(
             name=self.name,
-            parameters={param_key: DataEncoder().default(p) if hasattr(p, 'to_data') else p for param_key, p in
-                        self.parameters.items()},
+            parameters=self.parameters,
         )
-
-    @classmethod
-    def from_data(cls, data):
-        return cls(data['name'],
-                   {param_key: DataDecoder().object_hook(p) if hasattr(p, '__iter__') else p for param_key, p in
-                    data['parameters'].items()})
-
-    def to_data(self):
-        return self.data

From 1c92c817663faa37a95122b8db1169faaf4125e7 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 8 Apr 2021 07:41:26 +0200
Subject: [PATCH 04/28] add tests, and some fixes

---
 src/compas_fab/datastructures/__init__.py |   2 +
 src/compas_fab/datastructures/plan.py     |  95 ++++++++++++--
 tests/datastructures/test_plan.py         | 150 ++++++++++++++++++++++
 3 files changed, 237 insertions(+), 10 deletions(-)
 create mode 100644 tests/datastructures/test_plan.py

diff --git a/src/compas_fab/datastructures/__init__.py b/src/compas_fab/datastructures/__init__.py
index 0d45966f7..3ad67f372 100644
--- a/src/compas_fab/datastructures/__init__.py
+++ b/src/compas_fab/datastructures/__init__.py
@@ -1,5 +1,6 @@
 from .plan import (
     Action,
+    DependencyIdException,
     IntegerIdGenerator,
     Plan,
     PlannedAction
@@ -8,6 +9,7 @@
 
 __all__ = [
     'Action',
+    'DependencyIdException',
     'IntegerIdGenerator',
     'Plan',
     'PlannedAction',
diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 4b2869ec9..2834ae5be 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -2,22 +2,27 @@
 from __future__ import division
 from __future__ import print_function
 
+import json
 import threading
 from collections import OrderedDict
 from itertools import count
 
 from compas.base import Base
+from compas.datastructures import Datastructure
+from compas.utilities import DataDecoder
+from compas.utilities import DataEncoder
 
 
 __all__ = [
     'Action',
+    'DependencyIdException',
     'IntegerIdGenerator',
     'Plan',
     'PlannedAction',
 ]
 
 
-class IntegerIdGenerator(object):
+class IntegerIdGenerator(Base):
     """Generator object yielding integers sequentially in a thread safe manner.
 
     Parameters
@@ -26,12 +31,41 @@ class IntegerIdGenerator(object):
         First value to be yielded by the generator.
     """
     def __init__(self, start_value=1):
+        super(IntegerIdGenerator, self).__init__()
+        self.last_generated = start_value - 1
         self._lock = threading.Lock()
         self._generator = count(start_value)
 
     def __next__(self):
         with self._lock:
-            yield next(self._generator)
+            self.last_generated = next(self._generator)
+            return self.last_generated
+
+    @property
+    def data(self):
+        return {
+            'start_value': self.last_generated + 1
+        }
+
+    def to_data(self):
+        return self.data
+
+    @classmethod
+    def from_data(cls, data):
+        return cls(data['start_value'])
+
+    @classmethod
+    def from_json(cls, filepath):
+        with open(filepath, 'r') as fp:
+            data = json.load(fp, cls=DataDecoder)
+        return cls.from_data(data)
+
+    def to_json(self, filepath, pretty=False):
+        with open(filepath, 'w+') as f:
+            if pretty:
+                json.dump(self.data, f, sort_keys=True, indent=4, cls=DataEncoder)
+            else:
+                json.dump(self.data, f, cls=DataEncoder)
 
 
 class DependencyIdException(Exception):
@@ -47,7 +81,7 @@ def compose_message(invalid_ids, pa_id):
         return 'Found invalid dependency ids {}'.format(invalid_ids)
 
 
-class Plan(Base):
+class Plan(Datastructure):
     """Data structure for holding the information of a partially ordered plan
     (a directed acyclic graph).  The content of any event of the plan is contained
     in an :class:`compas_fab.datastructures.Action`. An event is scheduled and
@@ -66,16 +100,25 @@ class Plan(Base):
         methods.  Defaults to :class:`compas_fab.datastructures.IntegerIdGenerator`.
     """
     def __init__(self, planned_actions=None, id_generator=None):
+        super(Plan, self).__init__()
         planned_actions = planned_actions or []
-        self.planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
+        self.planned_actions = planned_actions
         if id_generator is None:
             try:
                 start_value = max(self.planned_actions.keys()) + 1 if self.planned_actions else 1
             except Exception:
-                raise Exception('Given ids not compatiable with default id_generator.')
+                raise Exception('Given ids not compatible with default id_generator.')
             id_generator = IntegerIdGenerator(start_value)
         self._id_generator = id_generator
 
+    @property
+    def planned_actions(self):
+        return self._planned_actions
+
+    @planned_actions.setter
+    def planned_actions(self, planned_actions):
+        self._planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
+
     def plan_action(self, action, dependency_ids):
         """Adds the action to the plan with the given dependencies,
         and generates an id for the newly planned action.
@@ -225,11 +268,21 @@ def helper(s, v, cur_id):
     @property
     def data(self):
         return dict(
-            planned_actions=list(self.planned_actions.values())
+            planned_actions=list(self.planned_actions.values()),
+            id_generator=self._id_generator,
         )
 
+    @data.setter
+    def data(self, data):
+        self.planned_actions = data['planned_actions']
+        self._id_generator = data['id_generator']
+
+    @classmethod
+    def from_data(cls, data):
+        return cls(**data)
+
 
-class PlannedAction(Base):
+class PlannedAction(Datastructure):
     """Represents an action which has been scheduled in a plan.
 
     Parameters
@@ -243,6 +296,7 @@ class PlannedAction(Base):
         The ids of the actions upon which `action` is dependent.
     """
     def __init__(self, action_id, action, dependency_ids):
+        super(PlannedAction, self).__init__()
         self.id = action_id
         self.action = action
         self.dependency_ids = dependency_ids
@@ -261,13 +315,24 @@ def __str__(self):
     @property
     def data(self):
         return dict(
-            id=self.id,
+            action_id=self.id,
             action=self.action,
-            dependency_ids=self.dependency_ids,
+            # sets are not json serializable
+            dependency_ids=list(self.dependency_ids),
         )
 
+    @data.setter
+    def data(self, data):
+        self.id = data['action_id']
+        self.action = data['action']
+        self.dependency_ids = data['dependency_ids']
 
-class Action(Base):
+    @classmethod
+    def from_data(cls, data):
+        return cls(**data)
+
+
+class Action(Datastructure):
     """Abstract representation of an event independent of its timing.
 
     Parameters
@@ -278,6 +343,7 @@ class Action(Base):
         Any other content associated to the action housed in key-value pairs.
     """
     def __init__(self, name, parameters=None):
+        super(Action, self).__init__()
         self.name = name
         self.parameters = parameters or {}
 
@@ -290,3 +356,12 @@ def data(self):
             name=self.name,
             parameters=self.parameters,
         )
+
+    @data.setter
+    def data(self, data):
+        self.name = data['name']
+        self.parameters = data['parameters']
+
+    @classmethod
+    def from_data(cls, data):
+        return cls(**data)
diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
new file mode 100644
index 000000000..c6e655a71
--- /dev/null
+++ b/tests/datastructures/test_plan.py
@@ -0,0 +1,150 @@
+from itertools import count
+
+import compas
+import pytest
+
+from compas.geometry import Frame
+from compas_fab.datastructures import Action
+from compas_fab.datastructures import IntegerIdGenerator
+from compas_fab.datastructures import Plan
+from compas_fab.datastructures import PlannedAction
+from compas_fab.datastructures import DependencyIdException
+
+
+def test_action_data():
+    name = "action"
+    parameters = {'param1': 1, 'param2': Frame.worldXY()}
+    action = Action(name, parameters)
+    other_action = Action.from_data(action.data)
+    assert action.name == other_action.name
+    assert action.parameters == other_action.parameters
+
+    action_json = compas.json_dumps(action.data)
+    other_action_data = compas.json_loads(action_json)
+    other_action = Action.from_data(other_action_data)
+    assert other_action.parameters['param2'] == Frame.worldXY()
+
+
+def test_planned_action_data():
+    action_id = 1
+    action = Action('action', {'param': 'hi there'})
+    dependency_ids = []
+    planned_action = PlannedAction(action_id, action, dependency_ids)
+    other_planned_action = PlannedAction.from_data(planned_action.data)
+    assert planned_action.id == other_planned_action.id
+    assert planned_action.action == other_planned_action.action
+    assert planned_action.dependency_ids == other_planned_action.dependency_ids
+
+    pa_json = compas.json_dumps(planned_action.data)
+    other_pa_data = compas.json_loads(pa_json)
+    other_pa = PlannedAction.from_data(other_pa_data)
+    assert other_pa.action.parameters['param'] == 'hi there'
+
+
+def test_integer_id_generator():
+    generator = IntegerIdGenerator()
+    assert next(generator) == 1
+    assert next(generator) == 2
+    generator_json = compas.json_dumps(generator.data)
+    generator_data = compas.json_loads(generator_json)
+    generator_reincarnate = IntegerIdGenerator.from_data(generator_data)
+    assert next(generator_reincarnate) == 3
+
+
+def test_plan():
+    plan = Plan()
+    assert len(plan.planned_actions) == 0
+
+
+def test_plan_generator_compatibility():
+    action = Action('action', {'param': 3})
+    pa = PlannedAction(1, action, [])
+    plan_a = Plan([pa])
+    assert plan_a.planned_actions[1].action.parameters['param'] == 3
+    assert next(plan_a._id_generator) == 2
+
+    pa = PlannedAction('a', action, [])
+    with pytest.raises(Exception):
+        _ = Plan([pa])
+
+
+def test_plan_with_custom_id_generator():
+    class CustomIdGenerator(object):
+        def __init__(self):
+            self._generator = count(ord('a'))
+
+        def __next__(self):
+            return chr(next(self._generator))
+
+    plan = Plan([], CustomIdGenerator())
+    action_a = Action('action_a', {})
+    plan.plan_action(action_a, [])
+    action_b = Action('action_b', {})
+    plan.plan_action(action_b, ['a'])
+    assert 'a' in plan.planned_actions
+    assert 'b' in plan.planned_actions
+    assert {'a'} == plan.planned_actions['b'].dependency_ids
+    plan.remove_action_by_id('a')
+    assert 'a' not in plan.planned_actions
+    assert 'b' in plan.planned_actions
+    assert set() == plan.planned_actions['b'].dependency_ids
+    action_c = Action('action_c', {})
+    plan.append_action(action_c)
+    assert 'c' in plan.planned_actions
+    assert {'b'} == plan.planned_actions['c'].dependency_ids
+
+
+def test_plan_data():
+    plan = Plan()
+    plan.append_action(Action('action_1'))
+    other_plan = Plan.from_data(plan.data)
+    assert plan.planned_actions.keys() == other_plan.planned_actions.keys()
+
+    # the data attributes point to the same generator,
+    # so no testing `append_action` yet
+    plan_json = compas.json_dumps(plan.data)
+    other_plan_data = compas.json_loads(plan_json)
+    other_plan = Plan.from_data(other_plan_data)
+    assert plan.planned_actions.keys() == other_plan.planned_actions.keys()
+
+    # now there are two generators
+    plan.append_action(Action('action_2'))
+    other_plan.append_action(Action('other_action_2'))
+    assert plan.planned_actions.keys() == other_plan.planned_actions.keys()
+
+
+def test_plan_action():
+    plan = Plan()
+    action_1_id = plan.plan_action(Action('action_1'), [])
+    assert action_1_id == 1
+    assert plan.planned_actions[action_1_id].action.name == 'action_1'
+    action_2_id = plan.plan_action(Action('action_2'), [action_1_id])
+    assert action_2_id == 2
+    assert plan.planned_actions[action_2_id].action.name == 'action_2'
+    with pytest.raises(DependencyIdException):
+        plan.plan_action(Action('action_impossible'), [5])
+
+
+def test_append_action():
+    pass
+
+
+def test_remove_action():
+    pass
+
+
+def test_check_dependency_ids():
+    pass
+
+
+def test_check_all_dependency_ids():
+    pass
+
+
+def test_check_for_cycles():
+    # depend on self
+    pass
+
+
+def test_linear_sort():
+    pass

From 031e3a940aa7f5529ebaecd6eee2a549f0fb295d Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 8 Apr 2021 11:51:12 +0200
Subject: [PATCH 05/28] more tests

---
 src/compas_fab/datastructures/plan.py |  1 +
 tests/datastructures/test_plan.py     | 90 ++++++++++++++++++++++++---
 2 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 2834ae5be..d69ef1faf 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -118,6 +118,7 @@ def planned_actions(self):
     @planned_actions.setter
     def planned_actions(self, planned_actions):
         self._planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
+        self.check_all_dependency_ids()
 
     def plan_action(self, action, dependency_ids):
         """Adds the action to the plan with the given dependencies,
diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
index c6e655a71..385de9425 100644
--- a/tests/datastructures/test_plan.py
+++ b/tests/datastructures/test_plan.py
@@ -126,25 +126,101 @@ def test_plan_action():
 
 
 def test_append_action():
-    pass
+    plan = Plan()
+    action_1_id = plan.append_action(Action('action_1'))
+    assert action_1_id == 1
+    assert plan.planned_actions[action_1_id].action.name == 'action_1'
+    assert plan.planned_actions[action_1_id].dependency_ids == set()
+    action_2_id = plan.append_action(Action('action_2'))
+    assert action_2_id == 2
+    assert plan.planned_actions[action_2_id].action.name == 'action_2'
+    assert plan.planned_actions[action_2_id].dependency_ids == {action_1_id}
+    with pytest.raises(DependencyIdException):
+        plan.plan_action(Action('action_impossible'), [5])
 
 
 def test_remove_action():
-    pass
+    plan = Plan()
+    action_1_id = plan.append_action(Action('action_1'))
+    action_2_id = plan.append_action(Action('action_2'))
+    action_3_id = plan.append_action(Action('action_3'))
+    assert plan.planned_actions[action_2_id].dependency_ids == {action_1_id}
+    assert plan.planned_actions[action_3_id].dependency_ids == {action_2_id}
+    plan.remove_action_by_id(action_2_id)
+    assert action_2_id not in plan.planned_actions
+    assert plan.planned_actions[action_3_id].dependency_ids == set()
 
 
 def test_check_dependency_ids():
-    pass
+    plan = Plan()
+    with pytest.raises(DependencyIdException):
+        plan.check_dependency_ids([1])
+    id_1 = plan.append_action(Action('action_1'))
+    plan.check_dependency_ids([id_1])
+    id_2 = plan.append_action(Action('action_2'))
+    plan.check_dependency_ids([id_1])
+    plan.check_dependency_ids([id_1, id_2])
+    with pytest.raises(DependencyIdException):
+        plan.check_dependency_ids([3])
+    with pytest.raises(DependencyIdException):
+        plan.check_dependency_ids([id_1, 5])
 
 
 def test_check_all_dependency_ids():
-    pass
+    plan = Plan()
+    plan.check_all_dependency_ids()
+    pa1 = PlannedAction(1, Action('action_1'), [])
+    pa2 = PlannedAction(2, Action('action_2'), [1])
+    plan.planned_actions[1] = pa1
+    plan.planned_actions[2] = pa2
+    plan.check_all_dependency_ids()
+    pa3 = PlannedAction(3, Action('action_3'), [2, 5])
+    plan.planned_actions[3] = pa3
+    with pytest.raises(DependencyIdException):
+        plan.check_all_dependency_ids()
+    with pytest.raises(DependencyIdException):
+        _ = Plan([pa1, pa2, pa3])
 
 
 def test_check_for_cycles():
-    # depend on self
-    pass
+    plan = Plan()
+    plan.append_action(Action('action_1'))
+    plan.planned_actions[1].dependency_ids.add(1)
+    with pytest.raises(Exception):
+        plan.check_for_cycles()
+
+    plan = Plan()
+    for i in range(5):
+        plan.append_action(Action('action_{}'.format(i + 1)))
+    plan.check_for_cycles()
+    plan.plan_action(Action('action_6'), [])
+    plan.check_for_cycles()
+    plan.planned_actions[4].dependency_ids.add(2)
+    plan.check_for_cycles()
+    plan.planned_actions[1].dependency_ids.add(3)
+    with pytest.raises(Exception):
+        plan.check_for_cycles()
+    plan.planned_actions[1].dependency_ids.discard(3)
+    plan.planned_actions[1].dependency_ids.add(2)
+    with pytest.raises(Exception):
+        plan.check_for_cycles()
 
 
 def test_linear_sort():
-    pass
+    plan = Plan()
+    ids = []
+    for i in range(5):
+        pa_id = plan.append_action(Action('action_{}'.format(i + 1)))
+        ids.append(pa_id)
+    sorted_actions = plan.linear_sort()
+    assert [pa.id for pa in sorted_actions] == ids
+
+    plan = Plan()
+    assert plan.linear_sort() == []
+
+    pa1_id = plan.plan_action(Action('action_1'), [])
+    pa2_id = plan.plan_action(Action('action_2'), [])
+    plan.plan_action(Action('action_3'), [pa1_id, pa2_id])
+    sorted_actions = plan.linear_sort()
+    sorted_ids = [pa.id for pa in sorted_actions]
+    assert sorted_ids == [1, 2, 3] or sorted_ids == [2, 1, 3]

From 87c1a67b8c516380e0de82e4c939dacc37b3234f Mon Sep 17 00:00:00 2001
From: beverlylytle <57254617+beverlylytle@users.noreply.github.com>
Date: Thu, 8 Apr 2021 11:55:41 +0200
Subject: [PATCH 06/28] Update src/compas_fab/datastructures/plan.py

---
 src/compas_fab/datastructures/plan.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index d69ef1faf..cbc043f5b 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -134,7 +134,7 @@ def plan_action(self, action, dependency_ids):
 
         Returns
         -------
-            The id of the newly planned.
+            The id of the newly planned action.
         """
         self.check_dependency_ids(dependency_ids)
         action_id = self._get_next_action_id()

From 68243214041623b1e2bf6970756e2e23b92c7c9d Mon Sep 17 00:00:00 2001
From: beverlylytle <57254617+beverlylytle@users.noreply.github.com>
Date: Thu, 8 Apr 2021 11:56:01 +0200
Subject: [PATCH 07/28] Update src/compas_fab/datastructures/plan.py

---
 src/compas_fab/datastructures/plan.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index cbc043f5b..65cf09dec 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -153,7 +153,7 @@ def append_action(self, action):
 
         Returns
         -------
-            The id of the newly planned.
+            The id of the newly planned action.
         """
         dependency_ids = set()
         if self.planned_actions:

From ed26c8405752f761cef8a356c3228f721f4a69ff Mon Sep 17 00:00:00 2001
From: beverlylytle <57254617+beverlylytle@users.noreply.github.com>
Date: Thu, 8 Apr 2021 11:58:29 +0200
Subject: [PATCH 08/28] Update src/compas_fab/datastructures/plan.py

---
 src/compas_fab/datastructures/plan.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 65cf09dec..8624c2615 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -173,7 +173,7 @@ def remove_action_by_id(self, action_id):
         Returns
         -------
         :class:`compas_fab.datastructure.Action`
-        The action of the removed :class:`compas_fab.datastructure.PlannedAction`
+            The action of the removed :class:`compas_fab.datastructure.PlannedAction`
         """
         planned_action = self.planned_actions.pop(action_id)
         for pa in self.planned_actions.values():

From b79cb473d3bf54944cd548dbad4359d5c1435f45 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 8 Apr 2021 12:08:40 +0200
Subject: [PATCH 09/28] add alias for ironpython

---
 src/compas_fab/datastructures/plan.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 8624c2615..5274294ea 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -41,6 +41,9 @@ def __next__(self):
             self.last_generated = next(self._generator)
             return self.last_generated
 
+    # alias for ironpython
+    next = __next__
+
     @property
     def data(self):
         return {

From f44387f256ac1d14d21a594314b322e9ee74337c Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 8 Apr 2021 12:25:54 +0200
Subject: [PATCH 10/28] more ipy aliases

---
 tests/datastructures/test_plan.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
index 385de9425..d83a37de2 100644
--- a/tests/datastructures/test_plan.py
+++ b/tests/datastructures/test_plan.py
@@ -76,6 +76,9 @@ def __init__(self):
         def __next__(self):
             return chr(next(self._generator))
 
+        # alias for ironpython
+        next = __next__
+
     plan = Plan([], CustomIdGenerator())
     action_a = Action('action_a', {})
     plan.plan_action(action_a, [])

From e00f63c7159f1edcd5d4b7c4a81ff8ad4fd9c718 Mon Sep 17 00:00:00 2001
From: beverlylytle <57254617+beverlylytle@users.noreply.github.com>
Date: Mon, 12 Apr 2021 08:05:38 +0200
Subject: [PATCH 11/28] Update CHANGELOG.rst

Co-authored-by: Gonzalo Casas <casas@arch.ethz.ch>
---
 CHANGELOG.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1fbe1dc0a..2d4c17073 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -13,7 +13,7 @@ Unreleased
 **Added**
 
 * Added support for MoveIt on ROS Noetic
-* In`compas.datastructures`, added `Plan`, `PlannedAction`, `Action` and `IntegerIdGenerator`
+* In ``compas.datastructures``, added ``Plan``, ``PlannedAction``, ``Action`` and ``IntegerIdGenerator``
 
 **Changed**
 

From e5a685af4ead2023a5d578863c7a1e9763050f24 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Mon, 12 Apr 2021 08:41:37 +0200
Subject: [PATCH 12/28] Use compas.json_<load/dump>

---
 src/compas_fab/datastructures/plan.py | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 5274294ea..eebd2915e 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -2,15 +2,13 @@
 from __future__ import division
 from __future__ import print_function
 
-import json
 import threading
 from collections import OrderedDict
 from itertools import count
 
+import compas
 from compas.base import Base
 from compas.datastructures import Datastructure
-from compas.utilities import DataDecoder
-from compas.utilities import DataEncoder
 
 
 __all__ = [
@@ -59,16 +57,11 @@ def from_data(cls, data):
 
     @classmethod
     def from_json(cls, filepath):
-        with open(filepath, 'r') as fp:
-            data = json.load(fp, cls=DataDecoder)
+        data = compas.json_load(filepath)
         return cls.from_data(data)
 
     def to_json(self, filepath, pretty=False):
-        with open(filepath, 'w+') as f:
-            if pretty:
-                json.dump(self.data, f, sort_keys=True, indent=4, cls=DataEncoder)
-            else:
-                json.dump(self.data, f, cls=DataEncoder)
+        compas.json_dump(self.data, filepath)
 
 
 class DependencyIdException(Exception):

From 164a68d1392ae3d3f4ebd1f2b2a86c310314dbae Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Mon, 12 Apr 2021 09:00:41 +0200
Subject: [PATCH 13/28] Inherit from Base rather than Datastructure

---
 src/compas_fab/datastructures/plan.py | 39 ++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index eebd2915e..7174f9a16 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -4,6 +4,7 @@
 
 import threading
 from collections import OrderedDict
+from copy import deepcopy
 from itertools import count
 
 import compas
@@ -60,7 +61,7 @@ def from_json(cls, filepath):
         data = compas.json_load(filepath)
         return cls.from_data(data)
 
-    def to_json(self, filepath, pretty=False):
+    def to_json(self, filepath):
         compas.json_dump(self.data, filepath)
 
 
@@ -279,7 +280,7 @@ def from_data(cls, data):
         return cls(**data)
 
 
-class PlannedAction(Datastructure):
+class PlannedAction(Base):
     """Represents an action which has been scheduled in a plan.
 
     Parameters
@@ -328,8 +329,24 @@ def data(self, data):
     def from_data(cls, data):
         return cls(**data)
 
+    def to_data(self):
+        return self.data
+
+    @classmethod
+    def from_json(cls, filepath):
+        data = compas.json_load(filepath)
+        return cls.from_data(data)
+
+    def to_json(self, filepath):
+        compas.json_dump(self.data, filepath)
+
+    def copy(self, cls=None):
+        if not cls:
+            cls = type(self)
+        return cls.from_data(deepcopy(self.data))
+
 
-class Action(Datastructure):
+class Action(Base):
     """Abstract representation of an event independent of its timing.
 
     Parameters
@@ -362,3 +379,19 @@ def data(self, data):
     @classmethod
     def from_data(cls, data):
         return cls(**data)
+
+    def to_data(self):
+        return self.data
+
+    @classmethod
+    def from_json(cls, filepath):
+        data = compas.json_load(filepath)
+        return cls.from_data(data)
+
+    def to_json(self, filepath):
+        compas.json_dump(self.data, filepath)
+
+    def copy(self, cls=None):
+        if not cls:
+            cls = type(self)
+        return cls.from_data(deepcopy(self.data))

From a88c502001739c07c7fbbd283b1a8a1517d1638e Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Mon, 12 Apr 2021 09:02:17 +0200
Subject: [PATCH 14/28] remove extraneous from_data

---
 src/compas_fab/datastructures/plan.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 7174f9a16..4b8474612 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -275,10 +275,6 @@ def data(self, data):
         self.planned_actions = data['planned_actions']
         self._id_generator = data['id_generator']
 
-    @classmethod
-    def from_data(cls, data):
-        return cls(**data)
-
 
 class PlannedAction(Base):
     """Represents an action which has been scheduled in a plan.

From ccbca4b7d5b8f7f693374d8db0b076254eab3efe Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Mon, 12 Apr 2021 09:30:36 +0200
Subject: [PATCH 15/28] Add module level docstring

---
 src/compas_fab/datastructures/__init__.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/compas_fab/datastructures/__init__.py b/src/compas_fab/datastructures/__init__.py
index 3ad67f372..71866f7ad 100644
--- a/src/compas_fab/datastructures/__init__.py
+++ b/src/compas_fab/datastructures/__init__.py
@@ -1,3 +1,25 @@
+"""
+********************************************************************************
+compas_fab.datastructures
+********************************************************************************
+
+.. currentmodule:: compas_fab.datastructures
+
+Plan
+-----
+
+.. autosummary::
+    :toctree: generated/
+    :nosignatures:
+
+    Action
+    DependencyIdException
+    IntegerIdGenerator
+    Plan
+    PlannedAction
+
+"""
+
 from .plan import (
     Action,
     DependencyIdException,

From d89ee63a45f2c4896bf0946d26de2c7ebbe12f2d Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 15 Apr 2021 13:54:53 +0200
Subject: [PATCH 16/28] use compas.datastructures.Graph

---
 src/compas_fab/datastructures/__init__.py |   5 +-
 src/compas_fab/datastructures/plan.py     | 265 ++++++++--------------
 tests/datastructures/test_plan.py         | 138 +++--------
 3 files changed, 136 insertions(+), 272 deletions(-)

diff --git a/src/compas_fab/datastructures/__init__.py b/src/compas_fab/datastructures/__init__.py
index 71866f7ad..b6dc06225 100644
--- a/src/compas_fab/datastructures/__init__.py
+++ b/src/compas_fab/datastructures/__init__.py
@@ -16,7 +16,6 @@
     DependencyIdException
     IntegerIdGenerator
     Plan
-    PlannedAction
 
 """
 
@@ -24,8 +23,7 @@
     Action,
     DependencyIdException,
     IntegerIdGenerator,
-    Plan,
-    PlannedAction
+    Plan
 )
 
 
@@ -34,5 +32,4 @@
     'DependencyIdException',
     'IntegerIdGenerator',
     'Plan',
-    'PlannedAction',
 ]
diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 4b8474612..55a73ab79 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -6,18 +6,21 @@
 from collections import OrderedDict
 from copy import deepcopy
 from itertools import count
+from networkx import all_topological_sorts
+from networkx import find_cycle
+from networkx import lexicographical_topological_sort
+from networkx import NetworkXNoCycle
 
 import compas
 from compas.base import Base
 from compas.datastructures import Datastructure
-
+from compas.datastructures import Graph
 
 __all__ = [
     'Action',
     'DependencyIdException',
     'IntegerIdGenerator',
     'Plan',
-    'PlannedAction',
 ]
 
 
@@ -79,43 +82,60 @@ def compose_message(invalid_ids, pa_id):
 
 
 class Plan(Datastructure):
-    """Data structure for holding the information of a partially ordered plan
-    (a directed acyclic graph).  The content of any event of the plan is contained
-    in an :class:`compas_fab.datastructures.Action`. An event is scheduled and
-    added to the plan through a :class:`compas_fab.datastructures.PlannedAction`.
-    The dependency ids of a planned action can be thought of as pointers to the
-    parents of that planned action.
-
-    Parameters
+    """Data structure extending :class:`compas.datastructures.Graph` for creating
+    and maintaining a partially ordered plan (directed acyclic graph).
+    The content of any event of the plan is contained in an
+    :class:`compas_fab.datastructures.Action`.  The dependency ids of a planned
+    action can be thought of as pointers to the parents of that planned action.
+    While actions can be added and removed using the methods of
+    :attr:`compas_fab.datastructures.Plan.graph`, it is strongly recommended
+    that the methods ``plan_action``, ``append_action`` and ``remove_action``
+    are used instead.
+
+    Attributes
     ----------
-    planned_actions : list of :class:`compas_fab.datastructures.PlannedAction`
-        The planned actions will be stored as an ordered dictionary, so their
-        ids should be distinct.  Defaults to an empty list.
+    graph : :class:`compas.datastructures.Graph
     id_generator : Generator[Hashable, None, None]
         Object which generates keys (via ``next()``) for
         :class:`compas_fab.datastructures.Action`s added using this object's
         methods.  Defaults to :class:`compas_fab.datastructures.IntegerIdGenerator`.
     """
-    def __init__(self, planned_actions=None, id_generator=None):
+    def __init__(self, id_generator=None):
         super(Plan, self).__init__()
-        planned_actions = planned_actions or []
-        self.planned_actions = planned_actions
-        if id_generator is None:
-            try:
-                start_value = max(self.planned_actions.keys()) + 1 if self.planned_actions else 1
-            except Exception:
-                raise Exception('Given ids not compatible with default id_generator.')
-            id_generator = IntegerIdGenerator(start_value)
-        self._id_generator = id_generator
+        self.graph = Graph()
+        self.graph.node = OrderedDict()
+        self._id_generator = id_generator or IntegerIdGenerator()
+
+    @property
+    def networkx(self):
+        """A new NetworkX DiGraph instance from ``graph``."""
+        return self.graph.to_networkx()
 
     @property
-    def planned_actions(self):
-        return self._planned_actions
+    def actions(self):
+        """A dictionary of id-:class:`compas_fab.datastructures.Action` pairs."""
+        return {action_id: self.get_action(action_id) for action_id in self.graph.nodes()}
 
-    @planned_actions.setter
-    def planned_actions(self, planned_actions):
-        self._planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
-        self.check_all_dependency_ids()
+    def get_action(self, action_id):
+        """Gets the action for the associated ``action_id``
+
+        Parameters
+        ----------
+        action_id : hashable
+
+        Returns
+        -------
+        :class:`compas_fab.datastructures.Action`
+        """
+        action = self.graph.node_attribute(action_id, 'action')
+        if action is None:
+            raise Exception("Action with id {} not found".format(action_id))
+        return action
+
+    def remove_action(self, action_id):
+        action = self.get_action(action_id)
+        self.graph.delete_node(action_id)
+        return action
 
     def plan_action(self, action, dependency_ids):
         """Adds the action to the plan with the given dependencies,
@@ -135,8 +155,9 @@ def plan_action(self, action, dependency_ids):
         """
         self.check_dependency_ids(dependency_ids)
         action_id = self._get_next_action_id()
-        planned_action = PlannedAction(action_id, action, dependency_ids)
-        self.planned_actions[action_id] = planned_action
+        self.graph.add_node(action_id, action=action)
+        for dependency_id in dependency_ids:
+            self.graph.add_edge(dependency_id, action_id)
         return action_id
 
     def append_action(self, action):
@@ -153,47 +174,44 @@ def append_action(self, action):
             The id of the newly planned action.
         """
         dependency_ids = set()
-        if self.planned_actions:
+        if self.graph.node:
             last_action_id = self._get_last_action_id()
             dependency_ids = {last_action_id}
         return self.plan_action(action, dependency_ids)
 
-    def remove_action_by_id(self, action_id):
-        """Removes the action with the given id from the plan and all its
-        dependencies.
+    def _get_last_action_id(self):
+        last_action_id, last_action_attrs = self.graph.node.popitem()
+        self.graph.node[last_action_id] = last_action_attrs
+        return last_action_id
+
+    def _get_next_action_id(self):
+        return next(self._id_generator)
+
+    def get_dependency_ids(self, action_id):
+        """Return the identifiers of actions upon which the action with id ``action_id`` is dependent.
 
         Parameters
         ----------
-        action_id : Hashable
-            Id of the planned action to be removed.
+        action_id : hashable
+            The identifier of the action.
 
         Returns
         -------
-        :class:`compas_fab.datastructure.Action`
-            The action of the removed :class:`compas_fab.datastructure.PlannedAction`
-        """
-        planned_action = self.planned_actions.pop(action_id)
-        for pa in self.planned_actions.values():
-            pa.dependency_ids.discard(action_id)
-        return planned_action.action
+        :obj:`list`
+            A list of action identifiers.
 
-    def _get_last_action_id(self):
-        last_action_id, last_action = self.planned_actions.popitem()
-        self.planned_actions[last_action_id] = last_action
-        return last_action_id
-
-    def _get_next_action_id(self):
-        return next(self._id_generator)
+        """
+        return self.graph.neighbors_in(action_id)
 
-    def check_dependency_ids(self, dependency_ids, planned_action_id=None):
+    def check_dependency_ids(self, dependency_ids, action_id=None):
         """Checks whether the given dependency ids exist in the plan.
 
         Parameters
         ----------
         dependency_ids : set or list
             The dependency ids to be validated.
-        planned_action_id : Hashable
-            The id of the associated planned action.  Used only in
+        action_id : hashable
+            The id of the associated action.  Used only in
             the error message.  Defaults to ``None``.
 
         Raises
@@ -201,9 +219,9 @@ def check_dependency_ids(self, dependency_ids, planned_action_id=None):
         :class:`compas_fab.datastructures.DependencyIdException`
         """
         dependency_ids = set(dependency_ids)
-        if not dependency_ids.issubset(self.planned_actions):
-            invalid_ids = dependency_ids.difference(self.planned_actions)
-            raise DependencyIdException(invalid_ids, planned_action_id)
+        if not dependency_ids.issubset(self.graph.node):
+            invalid_ids = dependency_ids.difference(self.graph.node)
+            raise DependencyIdException(invalid_ids, action_id)
 
     def check_all_dependency_ids(self):
         """Checks whether the dependency ids of all the planned actions
@@ -213,31 +231,18 @@ def check_all_dependency_ids(self):
         ------
         :class:`compas_fab.datastructures.DependencyIdException`
         """
-        for pa_id, planned_action in self.planned_actions.items():
-            self.check_dependency_ids(planned_action.dependency_ids, pa_id)
+        for action_id in self.actions:
+            self.check_dependency_ids(self.get_dependency_ids(action_id), action_id)
 
     def check_for_cycles(self):
         """"Checks whether cycles exist in the dependency graph."""
-        self.check_all_dependency_ids()
-
-        def helper(cur, v, r):
-            v[cur] = True
-            r[cur] = True
-            for dep_id in self.planned_actions[cur].dependency_ids:
-                if not v[dep_id]:
-                    helper(dep_id, v, r)
-                elif r[dep_id]:
-                    raise Exception("Cycle found with action ids {}".format([pa_id for pa_id, seen in r.items() if seen]))
-            r[cur] = False
-
-        visited = {pa_id: False for pa_id in self.planned_actions}
-        rec_dict = {pa_id: False for pa_id in self.planned_actions}
-
-        for pa_id in self.planned_actions:
-            if not visited[pa_id]:
-                helper(pa_id, visited, rec_dict)
-
-    def linear_sort(self):
+        try:
+            cycle = find_cycle(self.networkx)
+        except NetworkXNoCycle:
+            return
+        raise Exception("Cycle found with edges {}".format(cycle))  # !!!
+
+    def get_linear_sort(self):
         """Sorts the planned actions linearly respecting the dependency ids.
 
         Returns
@@ -245,103 +250,33 @@ def linear_sort(self):
         :obj:`list` of :class:`compas_fab.datastructure.Action`
         """
         self.check_for_cycles()
+        return [self.get_action(action_id) for action_id in lexicographical_topological_sort(self.networkx)]
 
-        def helper(s, v, cur_id):
-            v.add(cur_id)
-            action = self.planned_actions[cur_id]
-            for dep_id in action.dependency_ids:
-                if dep_id not in v:
-                    helper(s, v, dep_id)
-            s.append(action)
-
-        stack = []
-        visited = set()
-
-        for action_id in self.planned_actions:
-            if action_id not in visited:
-                helper(stack, visited, action_id)
+    def get_all_linear_sorts(self):
+        """Gets all possible linear sorts respecting the dependency ids.
 
-        return stack
+        Returns
+        -------
+        :obj:`list` of :obj:`list of :class:`compas_fab.datastructure.Action`
+        """
+        self.check_for_cycles()
+        return [[self.get_action(action_id) for action_id in sorting] for sorting in all_topological_sorts(self.networkx)]
 
     @property
     def data(self):
-        return dict(
-            planned_actions=list(self.planned_actions.values()),
-            id_generator=self._id_generator,
-        )
+        return {
+            'graph': self.graph,
+            'id_generator': self._id_generator,
+        }
 
     @data.setter
     def data(self, data):
-        self.planned_actions = data['planned_actions']
+        graph = data['graph']
+        graph.node = OrderedDict(graph.node)
+        self.graph = graph
         self._id_generator = data['id_generator']
 
 
-class PlannedAction(Base):
-    """Represents an action which has been scheduled in a plan.
-
-    Parameters
-    ----------
-    action_id : Hashable
-        An identifier of the action.  Used by other actions and the plan
-        it is associated with.
-    action : :class:`compas_fab.datastructures.Action`
-        The action to be planned.
-    dependency_ids : set or list
-        The ids of the actions upon which `action` is dependent.
-    """
-    def __init__(self, action_id, action, dependency_ids):
-        super(PlannedAction, self).__init__()
-        self.id = action_id
-        self.action = action
-        self.dependency_ids = dependency_ids
-
-    @property
-    def dependency_ids(self):
-        return self._dependency_ids
-
-    @dependency_ids.setter
-    def dependency_ids(self, value):
-        self._dependency_ids = set(value)
-
-    def __str__(self):
-        return 'PlannedAction<id={}, action={}>'.format(self.id, self.action)
-
-    @property
-    def data(self):
-        return dict(
-            action_id=self.id,
-            action=self.action,
-            # sets are not json serializable
-            dependency_ids=list(self.dependency_ids),
-        )
-
-    @data.setter
-    def data(self, data):
-        self.id = data['action_id']
-        self.action = data['action']
-        self.dependency_ids = data['dependency_ids']
-
-    @classmethod
-    def from_data(cls, data):
-        return cls(**data)
-
-    def to_data(self):
-        return self.data
-
-    @classmethod
-    def from_json(cls, filepath):
-        data = compas.json_load(filepath)
-        return cls.from_data(data)
-
-    def to_json(self, filepath):
-        compas.json_dump(self.data, filepath)
-
-    def copy(self, cls=None):
-        if not cls:
-            cls = type(self)
-        return cls.from_data(deepcopy(self.data))
-
-
 class Action(Base):
     """Abstract representation of an event independent of its timing.
 
diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
index d83a37de2..4bfd58564 100644
--- a/tests/datastructures/test_plan.py
+++ b/tests/datastructures/test_plan.py
@@ -7,7 +7,6 @@
 from compas_fab.datastructures import Action
 from compas_fab.datastructures import IntegerIdGenerator
 from compas_fab.datastructures import Plan
-from compas_fab.datastructures import PlannedAction
 from compas_fab.datastructures import DependencyIdException
 
 
@@ -25,22 +24,6 @@ def test_action_data():
     assert other_action.parameters['param2'] == Frame.worldXY()
 
 
-def test_planned_action_data():
-    action_id = 1
-    action = Action('action', {'param': 'hi there'})
-    dependency_ids = []
-    planned_action = PlannedAction(action_id, action, dependency_ids)
-    other_planned_action = PlannedAction.from_data(planned_action.data)
-    assert planned_action.id == other_planned_action.id
-    assert planned_action.action == other_planned_action.action
-    assert planned_action.dependency_ids == other_planned_action.dependency_ids
-
-    pa_json = compas.json_dumps(planned_action.data)
-    other_pa_data = compas.json_loads(pa_json)
-    other_pa = PlannedAction.from_data(other_pa_data)
-    assert other_pa.action.parameters['param'] == 'hi there'
-
-
 def test_integer_id_generator():
     generator = IntegerIdGenerator()
     assert next(generator) == 1
@@ -53,19 +36,15 @@ def test_integer_id_generator():
 
 def test_plan():
     plan = Plan()
-    assert len(plan.planned_actions) == 0
+    assert len(plan.actions) == 0
 
 
 def test_plan_generator_compatibility():
     action = Action('action', {'param': 3})
-    pa = PlannedAction(1, action, [])
-    plan_a = Plan([pa])
-    assert plan_a.planned_actions[1].action.parameters['param'] == 3
-    assert next(plan_a._id_generator) == 2
-
-    pa = PlannedAction('a', action, [])
-    with pytest.raises(Exception):
-        _ = Plan([pa])
+    plan = Plan()
+    action_id = plan.plan_action(action, [])
+    assert action_id == 1
+    assert next(plan._id_generator) == 2
 
 
 def test_plan_with_custom_id_generator():
@@ -79,51 +58,51 @@ def __next__(self):
         # alias for ironpython
         next = __next__
 
-    plan = Plan([], CustomIdGenerator())
+    plan = Plan(CustomIdGenerator())
     action_a = Action('action_a', {})
     plan.plan_action(action_a, [])
     action_b = Action('action_b', {})
     plan.plan_action(action_b, ['a'])
-    assert 'a' in plan.planned_actions
-    assert 'b' in plan.planned_actions
-    assert {'a'} == plan.planned_actions['b'].dependency_ids
-    plan.remove_action_by_id('a')
-    assert 'a' not in plan.planned_actions
-    assert 'b' in plan.planned_actions
-    assert set() == plan.planned_actions['b'].dependency_ids
+    assert 'a' in plan.actions
+    assert 'b' in plan.actions
+    assert ['a'] == plan.get_dependency_ids('b')
+    plan.remove_action('a')
+    assert 'a' not in plan.actions
+    assert 'b' in plan.actions
+    assert [] == plan.get_dependency_ids('b')
     action_c = Action('action_c', {})
     plan.append_action(action_c)
-    assert 'c' in plan.planned_actions
-    assert {'b'} == plan.planned_actions['c'].dependency_ids
+    assert 'c' in plan.actions
+    assert ['b'] == plan.get_dependency_ids('c')
 
 
 def test_plan_data():
     plan = Plan()
     plan.append_action(Action('action_1'))
     other_plan = Plan.from_data(plan.data)
-    assert plan.planned_actions.keys() == other_plan.planned_actions.keys()
+    assert plan.actions.keys() == other_plan.actions.keys()
 
     # the data attributes point to the same generator,
     # so no testing `append_action` yet
     plan_json = compas.json_dumps(plan.data)
     other_plan_data = compas.json_loads(plan_json)
     other_plan = Plan.from_data(other_plan_data)
-    assert plan.planned_actions.keys() == other_plan.planned_actions.keys()
+    assert plan.actions.keys() == other_plan.actions.keys()
 
     # now there are two generators
     plan.append_action(Action('action_2'))
     other_plan.append_action(Action('other_action_2'))
-    assert plan.planned_actions.keys() == other_plan.planned_actions.keys()
+    assert plan.actions.keys() == other_plan.actions.keys()
 
 
 def test_plan_action():
     plan = Plan()
     action_1_id = plan.plan_action(Action('action_1'), [])
     assert action_1_id == 1
-    assert plan.planned_actions[action_1_id].action.name == 'action_1'
+    assert plan.get_action(action_1_id).name == 'action_1'
     action_2_id = plan.plan_action(Action('action_2'), [action_1_id])
     assert action_2_id == 2
-    assert plan.planned_actions[action_2_id].action.name == 'action_2'
+    assert plan.get_action(action_2_id).name == 'action_2'
     with pytest.raises(DependencyIdException):
         plan.plan_action(Action('action_impossible'), [5])
 
@@ -132,12 +111,12 @@ def test_append_action():
     plan = Plan()
     action_1_id = plan.append_action(Action('action_1'))
     assert action_1_id == 1
-    assert plan.planned_actions[action_1_id].action.name == 'action_1'
-    assert plan.planned_actions[action_1_id].dependency_ids == set()
+    assert plan.get_action(action_1_id).name == 'action_1'
+    assert plan.get_dependency_ids(action_1_id) == []
     action_2_id = plan.append_action(Action('action_2'))
     assert action_2_id == 2
-    assert plan.planned_actions[action_2_id].action.name == 'action_2'
-    assert plan.planned_actions[action_2_id].dependency_ids == {action_1_id}
+    assert plan.get_action(action_2_id).name == 'action_2'
+    assert plan.get_dependency_ids(action_2_id) == [action_1_id]
     with pytest.raises(DependencyIdException):
         plan.plan_action(Action('action_impossible'), [5])
 
@@ -147,11 +126,11 @@ def test_remove_action():
     action_1_id = plan.append_action(Action('action_1'))
     action_2_id = plan.append_action(Action('action_2'))
     action_3_id = plan.append_action(Action('action_3'))
-    assert plan.planned_actions[action_2_id].dependency_ids == {action_1_id}
-    assert plan.planned_actions[action_3_id].dependency_ids == {action_2_id}
-    plan.remove_action_by_id(action_2_id)
-    assert action_2_id not in plan.planned_actions
-    assert plan.planned_actions[action_3_id].dependency_ids == set()
+    assert plan.get_dependency_ids(action_2_id) == [action_1_id]
+    assert plan.get_dependency_ids(action_3_id) == [action_2_id]
+    plan.remove_action(action_2_id)
+    assert action_2_id not in plan.actions
+    assert plan.get_dependency_ids(action_3_id) == []
 
 
 def test_check_dependency_ids():
@@ -172,58 +151,11 @@ def test_check_dependency_ids():
 def test_check_all_dependency_ids():
     plan = Plan()
     plan.check_all_dependency_ids()
-    pa1 = PlannedAction(1, Action('action_1'), [])
-    pa2 = PlannedAction(2, Action('action_2'), [1])
-    plan.planned_actions[1] = pa1
-    plan.planned_actions[2] = pa2
+    plan.plan_action(Action('action_1'), [])
+    plan.plan_action(Action('action_2'), [1])
     plan.check_all_dependency_ids()
-    pa3 = PlannedAction(3, Action('action_3'), [2, 5])
-    plan.planned_actions[3] = pa3
-    with pytest.raises(DependencyIdException):
-        plan.check_all_dependency_ids()
-    with pytest.raises(DependencyIdException):
-        _ = Plan([pa1, pa2, pa3])
-
-
-def test_check_for_cycles():
-    plan = Plan()
-    plan.append_action(Action('action_1'))
-    plan.planned_actions[1].dependency_ids.add(1)
-    with pytest.raises(Exception):
-        plan.check_for_cycles()
-
-    plan = Plan()
-    for i in range(5):
-        plan.append_action(Action('action_{}'.format(i + 1)))
-    plan.check_for_cycles()
-    plan.plan_action(Action('action_6'), [])
-    plan.check_for_cycles()
-    plan.planned_actions[4].dependency_ids.add(2)
-    plan.check_for_cycles()
-    plan.planned_actions[1].dependency_ids.add(3)
-    with pytest.raises(Exception):
-        plan.check_for_cycles()
-    plan.planned_actions[1].dependency_ids.discard(3)
-    plan.planned_actions[1].dependency_ids.add(2)
+    plan.graph.add_node(3, action=Action('action_3'))
+    plan.graph.add_edge(2, 3)
+    plan.graph.add_edge(5, 3)
     with pytest.raises(Exception):
-        plan.check_for_cycles()
-
-
-def test_linear_sort():
-    plan = Plan()
-    ids = []
-    for i in range(5):
-        pa_id = plan.append_action(Action('action_{}'.format(i + 1)))
-        ids.append(pa_id)
-    sorted_actions = plan.linear_sort()
-    assert [pa.id for pa in sorted_actions] == ids
-
-    plan = Plan()
-    assert plan.linear_sort() == []
-
-    pa1_id = plan.plan_action(Action('action_1'), [])
-    pa2_id = plan.plan_action(Action('action_2'), [])
-    plan.plan_action(Action('action_3'), [pa1_id, pa2_id])
-    sorted_actions = plan.linear_sort()
-    sorted_ids = [pa.id for pa in sorted_actions]
-    assert sorted_ids == [1, 2, 3] or sorted_ids == [2, 1, 3]
+        plan.check_all_dependency_ids()

From 93f395243b3c7f1c252dd9bc8b5290384fd7c8b9 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 15 Apr 2021 14:03:42 +0200
Subject: [PATCH 17/28] locally import networkx to make ipy happy

---
 src/compas_fab/datastructures/plan.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 55a73ab79..775d684a8 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -6,10 +6,6 @@
 from collections import OrderedDict
 from copy import deepcopy
 from itertools import count
-from networkx import all_topological_sorts
-from networkx import find_cycle
-from networkx import lexicographical_topological_sort
-from networkx import NetworkXNoCycle
 
 import compas
 from compas.base import Base
@@ -236,11 +232,13 @@ def check_all_dependency_ids(self):
 
     def check_for_cycles(self):
         """"Checks whether cycles exist in the dependency graph."""
+        from networkx import find_cycle
+        from networkx import NetworkXNoCycle
         try:
             cycle = find_cycle(self.networkx)
         except NetworkXNoCycle:
             return
-        raise Exception("Cycle found with edges {}".format(cycle))  # !!!
+        raise Exception("Cycle found with edges {}".format(cycle))
 
     def get_linear_sort(self):
         """Sorts the planned actions linearly respecting the dependency ids.
@@ -249,6 +247,7 @@ def get_linear_sort(self):
         -------
         :obj:`list` of :class:`compas_fab.datastructure.Action`
         """
+        from networkx import lexicographical_topological_sort
         self.check_for_cycles()
         return [self.get_action(action_id) for action_id in lexicographical_topological_sort(self.networkx)]
 
@@ -259,6 +258,7 @@ def get_all_linear_sorts(self):
         -------
         :obj:`list` of :obj:`list of :class:`compas_fab.datastructure.Action`
         """
+        from networkx import all_topological_sorts
         self.check_for_cycles()
         return [[self.get_action(action_id) for action_id in sorting] for sorting in all_topological_sorts(self.networkx)]
 

From 9651875927d54336dd71c4ba741d6e9ccff8a849 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 15 Apr 2021 14:05:09 +0200
Subject: [PATCH 18/28] fix changelog

---
 CHANGELOG.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 9685d1d59..30cac94b2 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -14,7 +14,7 @@ Unreleased
 
 * Added support for MoveIt on ROS Noetic
 * Added support for Python 3.9
-* In ``compas.datastructures``, added ``Plan``, ``PlannedAction``, ``Action`` and ``IntegerIdGenerator``
+* In ``compas.datastructures``, added ``Plan``, ``Action`` and ``IntegerIdGenerator``
 
 **Changed**
 

From f05da6f598cdf22e43c0533adec62374a5bd91bc Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Thu, 15 Apr 2021 16:53:56 +0200
Subject: [PATCH 19/28] extend test

---
 tests/datastructures/test_plan.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
index 4bfd58564..e3f5ef8e0 100644
--- a/tests/datastructures/test_plan.py
+++ b/tests/datastructures/test_plan.py
@@ -78,7 +78,7 @@ def __next__(self):
 
 def test_plan_data():
     plan = Plan()
-    plan.append_action(Action('action_1'))
+    plan.append_action(Action('action_1', {'param': Frame.worldXY()}))
     other_plan = Plan.from_data(plan.data)
     assert plan.actions.keys() == other_plan.actions.keys()
 
@@ -88,6 +88,7 @@ def test_plan_data():
     other_plan_data = compas.json_loads(plan_json)
     other_plan = Plan.from_data(other_plan_data)
     assert plan.actions.keys() == other_plan.actions.keys()
+    assert other_plan.get_action(1).parameters['param'] == Frame.worldXY()
 
     # now there are two generators
     plan.append_action(Action('action_2'))

From 8c8d637321ad971259da58e0ac8a2eb3879e5dbe Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Fri, 16 Apr 2021 11:00:37 +0200
Subject: [PATCH 20/28] better test

---
 src/compas_fab/datastructures/plan.py | 2 +-
 tests/datastructures/test_plan.py     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 775d684a8..743a493b2 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -256,7 +256,7 @@ def get_all_linear_sorts(self):
 
         Returns
         -------
-        :obj:`list` of :obj:`list of :class:`compas_fab.datastructure.Action`
+        :obj:`list` of :obj:`list` of :class:`compas_fab.datastructure.Action`
         """
         from networkx import all_topological_sorts
         self.check_for_cycles()
diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
index e3f5ef8e0..16140cd28 100644
--- a/tests/datastructures/test_plan.py
+++ b/tests/datastructures/test_plan.py
@@ -88,7 +88,7 @@ def test_plan_data():
     other_plan_data = compas.json_loads(plan_json)
     other_plan = Plan.from_data(other_plan_data)
     assert plan.actions.keys() == other_plan.actions.keys()
-    assert other_plan.get_action(1).parameters['param'] == Frame.worldXY()
+    assert isinstance(other_plan.get_action(1).parameters['param'], Frame)
 
     # now there are two generators
     plan.append_action(Action('action_2'))

From d4941f5371119377e18816c266378fdad22ebf74 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Tue, 20 Apr 2021 11:15:16 +0200
Subject: [PATCH 21/28] update to newest compas version

---
 setup.py                              |  2 +-
 src/compas_fab/datastructures/plan.py | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/setup.py b/setup.py
index a8ca29dfe..fcac671cf 100644
--- a/setup.py
+++ b/setup.py
@@ -10,7 +10,7 @@
 from setuptools import find_packages, setup
 
 requirements = [
-    'compas>=1.3,<2.0',
+    'compas>=1.5,<2.0',
     'roslibpy>=1.1.0',
     'pybullet',
     'pyserial',
diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/plan.py
index 743a493b2..e92de517d 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/plan.py
@@ -8,7 +8,7 @@
 from itertools import count
 
 import compas
-from compas.base import Base
+from compas.data import Data
 from compas.datastructures import Datastructure
 from compas.datastructures import Graph
 
@@ -20,7 +20,7 @@
 ]
 
 
-class IntegerIdGenerator(Base):
+class IntegerIdGenerator(Data):
     """Generator object yielding integers sequentially in a thread safe manner.
 
     Parameters
@@ -60,8 +60,8 @@ def from_json(cls, filepath):
         data = compas.json_load(filepath)
         return cls.from_data(data)
 
-    def to_json(self, filepath):
-        compas.json_dump(self.data, filepath)
+    def to_json(self, filepath, pretty=False):
+        compas.json_dump(self.data, filepath, pretty=pretty)
 
 
 class DependencyIdException(Exception):
@@ -277,7 +277,7 @@ def data(self, data):
         self._id_generator = data['id_generator']
 
 
-class Action(Base):
+class Action(Data):
     """Abstract representation of an event independent of its timing.
 
     Parameters
@@ -319,8 +319,8 @@ def from_json(cls, filepath):
         data = compas.json_load(filepath)
         return cls.from_data(data)
 
-    def to_json(self, filepath):
-        compas.json_dump(self.data, filepath)
+    def to_json(self, filepath, pretty=False):
+        compas.json_dump(self.data, filepath, pretty=pretty)
 
     def copy(self, cls=None):
         if not cls:

From 4fcb952bcd09d13d0817831308365f73cc32cd86 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Tue, 25 May 2021 13:40:12 +0200
Subject: [PATCH 22/28] Rename Plan to PartialOrder

---
 src/compas_fab/datastructures/__init__.py     | 10 ++++----
 .../{plan.py => partial_order.py}             |  8 +++----
 tests/datastructures/test_plan.py             | 24 +++++++++----------
 3 files changed, 21 insertions(+), 21 deletions(-)
 rename src/compas_fab/datastructures/{plan.py => partial_order.py} (98%)

diff --git a/src/compas_fab/datastructures/__init__.py b/src/compas_fab/datastructures/__init__.py
index b6dc06225..6c4fbe5e2 100644
--- a/src/compas_fab/datastructures/__init__.py
+++ b/src/compas_fab/datastructures/__init__.py
@@ -5,7 +5,7 @@
 
 .. currentmodule:: compas_fab.datastructures
 
-Plan
+PartialOrder
 -----
 
 .. autosummary::
@@ -15,15 +15,15 @@
     Action
     DependencyIdException
     IntegerIdGenerator
-    Plan
+    PartialOrder
 
 """
 
-from .plan import (
+from .partial_order import (
     Action,
     DependencyIdException,
     IntegerIdGenerator,
-    Plan
+    PartialOrder
 )
 
 
@@ -31,5 +31,5 @@
     'Action',
     'DependencyIdException',
     'IntegerIdGenerator',
-    'Plan',
+    'PartialOrder',
 ]
diff --git a/src/compas_fab/datastructures/plan.py b/src/compas_fab/datastructures/partial_order.py
similarity index 98%
rename from src/compas_fab/datastructures/plan.py
rename to src/compas_fab/datastructures/partial_order.py
index e92de517d..f94b1505c 100644
--- a/src/compas_fab/datastructures/plan.py
+++ b/src/compas_fab/datastructures/partial_order.py
@@ -16,7 +16,7 @@
     'Action',
     'DependencyIdException',
     'IntegerIdGenerator',
-    'Plan',
+    'PartialOrder',
 ]
 
 
@@ -77,14 +77,14 @@ def compose_message(invalid_ids, pa_id):
         return 'Found invalid dependency ids {}'.format(invalid_ids)
 
 
-class Plan(Datastructure):
+class PartialOrder(Datastructure):
     """Data structure extending :class:`compas.datastructures.Graph` for creating
     and maintaining a partially ordered plan (directed acyclic graph).
     The content of any event of the plan is contained in an
     :class:`compas_fab.datastructures.Action`.  The dependency ids of a planned
     action can be thought of as pointers to the parents of that planned action.
     While actions can be added and removed using the methods of
-    :attr:`compas_fab.datastructures.Plan.graph`, it is strongly recommended
+    :attr:`compas_fab.datastructures.PartialOrder.graph`, it is strongly recommended
     that the methods ``plan_action``, ``append_action`` and ``remove_action``
     are used instead.
 
@@ -97,7 +97,7 @@ class Plan(Datastructure):
         methods.  Defaults to :class:`compas_fab.datastructures.IntegerIdGenerator`.
     """
     def __init__(self, id_generator=None):
-        super(Plan, self).__init__()
+        super(PartialOrder, self).__init__()
         self.graph = Graph()
         self.graph.node = OrderedDict()
         self._id_generator = id_generator or IntegerIdGenerator()
diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
index 16140cd28..3c44f5d5a 100644
--- a/tests/datastructures/test_plan.py
+++ b/tests/datastructures/test_plan.py
@@ -6,7 +6,7 @@
 from compas.geometry import Frame
 from compas_fab.datastructures import Action
 from compas_fab.datastructures import IntegerIdGenerator
-from compas_fab.datastructures import Plan
+from compas_fab.datastructures import PartialOrder
 from compas_fab.datastructures import DependencyIdException
 
 
@@ -35,13 +35,13 @@ def test_integer_id_generator():
 
 
 def test_plan():
-    plan = Plan()
+    plan = PartialOrder()
     assert len(plan.actions) == 0
 
 
 def test_plan_generator_compatibility():
     action = Action('action', {'param': 3})
-    plan = Plan()
+    plan = PartialOrder()
     action_id = plan.plan_action(action, [])
     assert action_id == 1
     assert next(plan._id_generator) == 2
@@ -58,7 +58,7 @@ def __next__(self):
         # alias for ironpython
         next = __next__
 
-    plan = Plan(CustomIdGenerator())
+    plan = PartialOrder(CustomIdGenerator())
     action_a = Action('action_a', {})
     plan.plan_action(action_a, [])
     action_b = Action('action_b', {})
@@ -77,16 +77,16 @@ def __next__(self):
 
 
 def test_plan_data():
-    plan = Plan()
+    plan = PartialOrder()
     plan.append_action(Action('action_1', {'param': Frame.worldXY()}))
-    other_plan = Plan.from_data(plan.data)
+    other_plan = PartialOrder.from_data(plan.data)
     assert plan.actions.keys() == other_plan.actions.keys()
 
     # the data attributes point to the same generator,
     # so no testing `append_action` yet
     plan_json = compas.json_dumps(plan.data)
     other_plan_data = compas.json_loads(plan_json)
-    other_plan = Plan.from_data(other_plan_data)
+    other_plan = PartialOrder.from_data(other_plan_data)
     assert plan.actions.keys() == other_plan.actions.keys()
     assert isinstance(other_plan.get_action(1).parameters['param'], Frame)
 
@@ -97,7 +97,7 @@ def test_plan_data():
 
 
 def test_plan_action():
-    plan = Plan()
+    plan = PartialOrder()
     action_1_id = plan.plan_action(Action('action_1'), [])
     assert action_1_id == 1
     assert plan.get_action(action_1_id).name == 'action_1'
@@ -109,7 +109,7 @@ def test_plan_action():
 
 
 def test_append_action():
-    plan = Plan()
+    plan = PartialOrder()
     action_1_id = plan.append_action(Action('action_1'))
     assert action_1_id == 1
     assert plan.get_action(action_1_id).name == 'action_1'
@@ -123,7 +123,7 @@ def test_append_action():
 
 
 def test_remove_action():
-    plan = Plan()
+    plan = PartialOrder()
     action_1_id = plan.append_action(Action('action_1'))
     action_2_id = plan.append_action(Action('action_2'))
     action_3_id = plan.append_action(Action('action_3'))
@@ -135,7 +135,7 @@ def test_remove_action():
 
 
 def test_check_dependency_ids():
-    plan = Plan()
+    plan = PartialOrder()
     with pytest.raises(DependencyIdException):
         plan.check_dependency_ids([1])
     id_1 = plan.append_action(Action('action_1'))
@@ -150,7 +150,7 @@ def test_check_dependency_ids():
 
 
 def test_check_all_dependency_ids():
-    plan = Plan()
+    plan = PartialOrder()
     plan.check_all_dependency_ids()
     plan.plan_action(Action('action_1'), [])
     plan.plan_action(Action('action_2'), [1])

From 829926ca6697a4da8d37fbb2da51219cd992d7d7 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Tue, 25 May 2021 14:11:32 +0200
Subject: [PATCH 23/28] Rename more stuff

---
 src/compas_fab/__init__.py                    |   1 +
 .../datastructures/partial_order.py           |   4 +-
 tests/datastructures/test_partial_order.py    | 162 ++++++++++++++++++
 tests/datastructures/test_plan.py             | 162 ------------------
 4 files changed, 165 insertions(+), 164 deletions(-)
 create mode 100644 tests/datastructures/test_partial_order.py
 delete mode 100644 tests/datastructures/test_plan.py

diff --git a/src/compas_fab/__init__.py b/src/compas_fab/__init__.py
index d3da99b7a..0a8a9b52d 100644
--- a/src/compas_fab/__init__.py
+++ b/src/compas_fab/__init__.py
@@ -14,6 +14,7 @@
 
     compas_fab.backends
     compas_fab.robots
+    compas_fab.datastructures
     compas_fab.utilities
     compas_fab.sensors
     compas_fab.blender
diff --git a/src/compas_fab/datastructures/partial_order.py b/src/compas_fab/datastructures/partial_order.py
index f94b1505c..2b9f8ad6b 100644
--- a/src/compas_fab/datastructures/partial_order.py
+++ b/src/compas_fab/datastructures/partial_order.py
@@ -133,7 +133,7 @@ def remove_action(self, action_id):
         self.graph.delete_node(action_id)
         return action
 
-    def plan_action(self, action, dependency_ids):
+    def add_action(self, action, dependency_ids):
         """Adds the action to the plan with the given dependencies,
         and generates an id for the newly planned action.
 
@@ -173,7 +173,7 @@ def append_action(self, action):
         if self.graph.node:
             last_action_id = self._get_last_action_id()
             dependency_ids = {last_action_id}
-        return self.plan_action(action, dependency_ids)
+        return self.add_action(action, dependency_ids)
 
     def _get_last_action_id(self):
         last_action_id, last_action_attrs = self.graph.node.popitem()
diff --git a/tests/datastructures/test_partial_order.py b/tests/datastructures/test_partial_order.py
new file mode 100644
index 000000000..e6ddff5e1
--- /dev/null
+++ b/tests/datastructures/test_partial_order.py
@@ -0,0 +1,162 @@
+from itertools import count
+
+import compas
+import pytest
+
+from compas.geometry import Frame
+from compas_fab.datastructures import Action
+from compas_fab.datastructures import IntegerIdGenerator
+from compas_fab.datastructures import PartialOrder
+from compas_fab.datastructures import DependencyIdException
+
+
+def test_action_data():
+    name = "action"
+    parameters = {'param1': 1, 'param2': Frame.worldXY()}
+    action = Action(name, parameters)
+    other_action = Action.from_data(action.data)
+    assert action.name == other_action.name
+    assert action.parameters == other_action.parameters
+
+    action_json = compas.json_dumps(action.data)
+    other_action_data = compas.json_loads(action_json)
+    other_action = Action.from_data(other_action_data)
+    assert other_action.parameters['param2'] == Frame.worldXY()
+
+
+def test_integer_id_generator():
+    generator = IntegerIdGenerator()
+    assert next(generator) == 1
+    assert next(generator) == 2
+    generator_json = compas.json_dumps(generator.data)
+    generator_data = compas.json_loads(generator_json)
+    generator_reincarnate = IntegerIdGenerator.from_data(generator_data)
+    assert next(generator_reincarnate) == 3
+
+
+def test_partial_order():
+    partial_order = PartialOrder()
+    assert len(partial_order.actions) == 0
+
+
+def test_partial_order_generator_compatibility():
+    action = Action('action', {'param': 3})
+    partial_order = PartialOrder()
+    action_id = partial_order.add_action(action, [])
+    assert action_id == 1
+    assert next(partial_order._id_generator) == 2
+
+
+def test_partial_order_with_custom_id_generator():
+    class CustomIdGenerator(object):
+        def __init__(self):
+            self._generator = count(ord('a'))
+
+        def __next__(self):
+            return chr(next(self._generator))
+
+        # alias for ironpython
+        next = __next__
+
+    partial_order = PartialOrder(CustomIdGenerator())
+    action_a = Action('action_a', {})
+    partial_order.add_action(action_a, [])
+    action_b = Action('action_b', {})
+    partial_order.add_action(action_b, ['a'])
+    assert 'a' in partial_order.actions
+    assert 'b' in partial_order.actions
+    assert ['a'] == partial_order.get_dependency_ids('b')
+    partial_order.remove_action('a')
+    assert 'a' not in partial_order.actions
+    assert 'b' in partial_order.actions
+    assert [] == partial_order.get_dependency_ids('b')
+    action_c = Action('action_c', {})
+    partial_order.append_action(action_c)
+    assert 'c' in partial_order.actions
+    assert ['b'] == partial_order.get_dependency_ids('c')
+
+
+def test_partial_order_data():
+    partial_order = PartialOrder()
+    partial_order.append_action(Action('action_1', {'param': Frame.worldXY()}))
+    other_partial_order = PartialOrder.from_data(partial_order.data)
+    assert partial_order.actions.keys() == other_partial_order.actions.keys()
+
+    # the data attributes point to the same generator,
+    # so no testing `append_action` yet
+    partial_order_json = compas.json_dumps(partial_order.data)
+    other_partial_order_data = compas.json_loads(partial_order_json)
+    other_partial_order = PartialOrder.from_data(other_partial_order_data)
+    assert partial_order.actions.keys() == other_partial_order.actions.keys()
+    assert isinstance(other_partial_order.get_action(1).parameters['param'], Frame)
+
+    # now there are two generators
+    partial_order.append_action(Action('action_2'))
+    other_partial_order.append_action(Action('other_action_2'))
+    assert partial_order.actions.keys() == other_partial_order.actions.keys()
+
+
+def test_partial_order_action():
+    partial_order = PartialOrder()
+    action_1_id = partial_order.add_action(Action('action_1'), [])
+    assert action_1_id == 1
+    assert partial_order.get_action(action_1_id).name == 'action_1'
+    action_2_id = partial_order.add_action(Action('action_2'), [action_1_id])
+    assert action_2_id == 2
+    assert partial_order.get_action(action_2_id).name == 'action_2'
+    with pytest.raises(DependencyIdException):
+        partial_order.add_action(Action('action_impossible'), [5])
+
+
+def test_append_action():
+    partial_order = PartialOrder()
+    action_1_id = partial_order.append_action(Action('action_1'))
+    assert action_1_id == 1
+    assert partial_order.get_action(action_1_id).name == 'action_1'
+    assert partial_order.get_dependency_ids(action_1_id) == []
+    action_2_id = partial_order.append_action(Action('action_2'))
+    assert action_2_id == 2
+    assert partial_order.get_action(action_2_id).name == 'action_2'
+    assert partial_order.get_dependency_ids(action_2_id) == [action_1_id]
+    with pytest.raises(DependencyIdException):
+        partial_order.add_action(Action('action_impossible'), [5])
+
+
+def test_remove_action():
+    partial_order = PartialOrder()
+    action_1_id = partial_order.append_action(Action('action_1'))
+    action_2_id = partial_order.append_action(Action('action_2'))
+    action_3_id = partial_order.append_action(Action('action_3'))
+    assert partial_order.get_dependency_ids(action_2_id) == [action_1_id]
+    assert partial_order.get_dependency_ids(action_3_id) == [action_2_id]
+    partial_order.remove_action(action_2_id)
+    assert action_2_id not in partial_order.actions
+    assert partial_order.get_dependency_ids(action_3_id) == []
+
+
+def test_check_dependency_ids():
+    partial_order = PartialOrder()
+    with pytest.raises(DependencyIdException):
+        partial_order.check_dependency_ids([1])
+    id_1 = partial_order.append_action(Action('action_1'))
+    partial_order.check_dependency_ids([id_1])
+    id_2 = partial_order.append_action(Action('action_2'))
+    partial_order.check_dependency_ids([id_1])
+    partial_order.check_dependency_ids([id_1, id_2])
+    with pytest.raises(DependencyIdException):
+        partial_order.check_dependency_ids([3])
+    with pytest.raises(DependencyIdException):
+        partial_order.check_dependency_ids([id_1, 5])
+
+
+def test_check_all_dependency_ids():
+    partial_order = PartialOrder()
+    partial_order.check_all_dependency_ids()
+    partial_order.add_action(Action('action_1'), [])
+    partial_order.add_action(Action('action_2'), [1])
+    partial_order.check_all_dependency_ids()
+    partial_order.graph.add_node(3, action=Action('action_3'))
+    partial_order.graph.add_edge(2, 3)
+    partial_order.graph.add_edge(5, 3)
+    with pytest.raises(Exception):
+        partial_order.check_all_dependency_ids()
diff --git a/tests/datastructures/test_plan.py b/tests/datastructures/test_plan.py
deleted file mode 100644
index 3c44f5d5a..000000000
--- a/tests/datastructures/test_plan.py
+++ /dev/null
@@ -1,162 +0,0 @@
-from itertools import count
-
-import compas
-import pytest
-
-from compas.geometry import Frame
-from compas_fab.datastructures import Action
-from compas_fab.datastructures import IntegerIdGenerator
-from compas_fab.datastructures import PartialOrder
-from compas_fab.datastructures import DependencyIdException
-
-
-def test_action_data():
-    name = "action"
-    parameters = {'param1': 1, 'param2': Frame.worldXY()}
-    action = Action(name, parameters)
-    other_action = Action.from_data(action.data)
-    assert action.name == other_action.name
-    assert action.parameters == other_action.parameters
-
-    action_json = compas.json_dumps(action.data)
-    other_action_data = compas.json_loads(action_json)
-    other_action = Action.from_data(other_action_data)
-    assert other_action.parameters['param2'] == Frame.worldXY()
-
-
-def test_integer_id_generator():
-    generator = IntegerIdGenerator()
-    assert next(generator) == 1
-    assert next(generator) == 2
-    generator_json = compas.json_dumps(generator.data)
-    generator_data = compas.json_loads(generator_json)
-    generator_reincarnate = IntegerIdGenerator.from_data(generator_data)
-    assert next(generator_reincarnate) == 3
-
-
-def test_plan():
-    plan = PartialOrder()
-    assert len(plan.actions) == 0
-
-
-def test_plan_generator_compatibility():
-    action = Action('action', {'param': 3})
-    plan = PartialOrder()
-    action_id = plan.plan_action(action, [])
-    assert action_id == 1
-    assert next(plan._id_generator) == 2
-
-
-def test_plan_with_custom_id_generator():
-    class CustomIdGenerator(object):
-        def __init__(self):
-            self._generator = count(ord('a'))
-
-        def __next__(self):
-            return chr(next(self._generator))
-
-        # alias for ironpython
-        next = __next__
-
-    plan = PartialOrder(CustomIdGenerator())
-    action_a = Action('action_a', {})
-    plan.plan_action(action_a, [])
-    action_b = Action('action_b', {})
-    plan.plan_action(action_b, ['a'])
-    assert 'a' in plan.actions
-    assert 'b' in plan.actions
-    assert ['a'] == plan.get_dependency_ids('b')
-    plan.remove_action('a')
-    assert 'a' not in plan.actions
-    assert 'b' in plan.actions
-    assert [] == plan.get_dependency_ids('b')
-    action_c = Action('action_c', {})
-    plan.append_action(action_c)
-    assert 'c' in plan.actions
-    assert ['b'] == plan.get_dependency_ids('c')
-
-
-def test_plan_data():
-    plan = PartialOrder()
-    plan.append_action(Action('action_1', {'param': Frame.worldXY()}))
-    other_plan = PartialOrder.from_data(plan.data)
-    assert plan.actions.keys() == other_plan.actions.keys()
-
-    # the data attributes point to the same generator,
-    # so no testing `append_action` yet
-    plan_json = compas.json_dumps(plan.data)
-    other_plan_data = compas.json_loads(plan_json)
-    other_plan = PartialOrder.from_data(other_plan_data)
-    assert plan.actions.keys() == other_plan.actions.keys()
-    assert isinstance(other_plan.get_action(1).parameters['param'], Frame)
-
-    # now there are two generators
-    plan.append_action(Action('action_2'))
-    other_plan.append_action(Action('other_action_2'))
-    assert plan.actions.keys() == other_plan.actions.keys()
-
-
-def test_plan_action():
-    plan = PartialOrder()
-    action_1_id = plan.plan_action(Action('action_1'), [])
-    assert action_1_id == 1
-    assert plan.get_action(action_1_id).name == 'action_1'
-    action_2_id = plan.plan_action(Action('action_2'), [action_1_id])
-    assert action_2_id == 2
-    assert plan.get_action(action_2_id).name == 'action_2'
-    with pytest.raises(DependencyIdException):
-        plan.plan_action(Action('action_impossible'), [5])
-
-
-def test_append_action():
-    plan = PartialOrder()
-    action_1_id = plan.append_action(Action('action_1'))
-    assert action_1_id == 1
-    assert plan.get_action(action_1_id).name == 'action_1'
-    assert plan.get_dependency_ids(action_1_id) == []
-    action_2_id = plan.append_action(Action('action_2'))
-    assert action_2_id == 2
-    assert plan.get_action(action_2_id).name == 'action_2'
-    assert plan.get_dependency_ids(action_2_id) == [action_1_id]
-    with pytest.raises(DependencyIdException):
-        plan.plan_action(Action('action_impossible'), [5])
-
-
-def test_remove_action():
-    plan = PartialOrder()
-    action_1_id = plan.append_action(Action('action_1'))
-    action_2_id = plan.append_action(Action('action_2'))
-    action_3_id = plan.append_action(Action('action_3'))
-    assert plan.get_dependency_ids(action_2_id) == [action_1_id]
-    assert plan.get_dependency_ids(action_3_id) == [action_2_id]
-    plan.remove_action(action_2_id)
-    assert action_2_id not in plan.actions
-    assert plan.get_dependency_ids(action_3_id) == []
-
-
-def test_check_dependency_ids():
-    plan = PartialOrder()
-    with pytest.raises(DependencyIdException):
-        plan.check_dependency_ids([1])
-    id_1 = plan.append_action(Action('action_1'))
-    plan.check_dependency_ids([id_1])
-    id_2 = plan.append_action(Action('action_2'))
-    plan.check_dependency_ids([id_1])
-    plan.check_dependency_ids([id_1, id_2])
-    with pytest.raises(DependencyIdException):
-        plan.check_dependency_ids([3])
-    with pytest.raises(DependencyIdException):
-        plan.check_dependency_ids([id_1, 5])
-
-
-def test_check_all_dependency_ids():
-    plan = PartialOrder()
-    plan.check_all_dependency_ids()
-    plan.plan_action(Action('action_1'), [])
-    plan.plan_action(Action('action_2'), [1])
-    plan.check_all_dependency_ids()
-    plan.graph.add_node(3, action=Action('action_3'))
-    plan.graph.add_edge(2, 3)
-    plan.graph.add_edge(5, 3)
-    with pytest.raises(Exception):
-        plan.check_all_dependency_ids()

From ab10364dd892ca838d9fc68e4e40601646d7441b Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Wed, 26 May 2021 08:10:58 +0200
Subject: [PATCH 24/28] Change test to make ironpython happy

---
 tests/datastructures/test_partial_order.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/datastructures/test_partial_order.py b/tests/datastructures/test_partial_order.py
index e6ddff5e1..86525dc20 100644
--- a/tests/datastructures/test_partial_order.py
+++ b/tests/datastructures/test_partial_order.py
@@ -88,7 +88,8 @@ def test_partial_order_data():
     other_partial_order_data = compas.json_loads(partial_order_json)
     other_partial_order = PartialOrder.from_data(other_partial_order_data)
     assert partial_order.actions.keys() == other_partial_order.actions.keys()
-    assert isinstance(other_partial_order.get_action(1).parameters['param'], Frame)
+    other_frame = other_partial_order.get_action(1).parameters['param']
+    assert other_frame == Frame.worldXY()
 
     # now there are two generators
     partial_order.append_action(Action('action_2'))

From 23ae4751e4a46da58b5d3820b3ca3fe61a785a90 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Wed, 26 May 2021 10:56:54 +0200
Subject: [PATCH 25/28] Add wikipedia link

---
 src/compas_fab/datastructures/partial_order.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/compas_fab/datastructures/partial_order.py b/src/compas_fab/datastructures/partial_order.py
index 2b9f8ad6b..d7d90bf9b 100644
--- a/src/compas_fab/datastructures/partial_order.py
+++ b/src/compas_fab/datastructures/partial_order.py
@@ -95,6 +95,10 @@ class PartialOrder(Datastructure):
         Object which generates keys (via ``next()``) for
         :class:`compas_fab.datastructures.Action`s added using this object's
         methods.  Defaults to :class:`compas_fab.datastructures.IntegerIdGenerator`.
+
+    Notes
+    -----
+    See <https://en.wikipedia.org/wiki/Partial-order_planning>.
     """
     def __init__(self, id_generator=None):
         super(PartialOrder, self).__init__()

From 07268fd94228afcd1eaa9ea7233c4f0e7ed57441 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Wed, 26 May 2021 12:54:25 +0200
Subject: [PATCH 26/28] Added first draft of accept methods

---
 src/compas_fab/datastructures/partial_order.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/compas_fab/datastructures/partial_order.py b/src/compas_fab/datastructures/partial_order.py
index d7d90bf9b..d1b95bc65 100644
--- a/src/compas_fab/datastructures/partial_order.py
+++ b/src/compas_fab/datastructures/partial_order.py
@@ -266,6 +266,20 @@ def get_all_linear_sorts(self):
         self.check_for_cycles()
         return [[self.get_action(action_id) for action_id in sorting] for sorting in all_topological_sorts(self.networkx)]
 
+    def accept(self, visitor, source_first=True):
+        sources = [key for key in self.graph.nodes() if len(self.graph.neighbors_in(key)) == 0]
+        for source in sources:
+            self._accept(source, visitor, source_first)
+
+    def _accept(self, key, visitor, source_first):
+        action = self.graph.node_attribute(key, 'action')
+        if source_first:
+            action.accept(visitor)
+        for child in self.graph.neighbors_out(key):
+            self._accept(child, visitor, source_first)
+        if not source_first:
+            action.accept(visitor)
+
     @property
     def data(self):
         return {
@@ -330,3 +344,6 @@ def copy(self, cls=None):
         if not cls:
             cls = type(self)
         return cls.from_data(deepcopy(self.data))
+
+    def accept(self, visitor):
+        raise NotImplementedError

From a5341ad6bb5c7a1ded4b5ce1f2adb986e9cd48f3 Mon Sep 17 00:00:00 2001
From: beverly <lytle@arch.ethz.ch>
Date: Wed, 26 May 2021 12:58:09 +0200
Subject: [PATCH 27/28] Yeah, I remember how my code works

---
 src/compas_fab/datastructures/partial_order.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compas_fab/datastructures/partial_order.py b/src/compas_fab/datastructures/partial_order.py
index d1b95bc65..067695f53 100644
--- a/src/compas_fab/datastructures/partial_order.py
+++ b/src/compas_fab/datastructures/partial_order.py
@@ -272,7 +272,7 @@ def accept(self, visitor, source_first=True):
             self._accept(source, visitor, source_first)
 
     def _accept(self, key, visitor, source_first):
-        action = self.graph.node_attribute(key, 'action')
+        action = self.get_action(key)
         if source_first:
             action.accept(visitor)
         for child in self.graph.neighbors_out(key):

From d28fb49479490d0406870ef349c8e10e83d0ac4a Mon Sep 17 00:00:00 2001
From: beverlylytle <57254617+beverlylytle@users.noreply.github.com>
Date: Wed, 26 May 2021 16:41:41 +0200
Subject: [PATCH 28/28] Update src/compas_fab/datastructures/partial_order.py

---
 src/compas_fab/datastructures/partial_order.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compas_fab/datastructures/partial_order.py b/src/compas_fab/datastructures/partial_order.py
index 067695f53..c85f1627a 100644
--- a/src/compas_fab/datastructures/partial_order.py
+++ b/src/compas_fab/datastructures/partial_order.py
@@ -85,7 +85,7 @@ class PartialOrder(Datastructure):
     action can be thought of as pointers to the parents of that planned action.
     While actions can be added and removed using the methods of
     :attr:`compas_fab.datastructures.PartialOrder.graph`, it is strongly recommended
-    that the methods ``plan_action``, ``append_action`` and ``remove_action``
+    that the methods ``add_action``, ``append_action`` and ``remove_action``
     are used instead.
 
     Attributes