Fix review comments. Update API

Author: pockerman

.gitignore

@@ -1,2 +1,5 @@
 src/preprocessor/__pycache__/
 src/exceptions/__pycache__/
+src/utils/__pycache__/
+src/tests/.pytest_cache/
+src/spaces/__pycache__/

src/datasets/dataset_distances.py

@@ -11,6 +11,15 @@
 
 
 def lp_distance(ds1: DataSet, ds2: DataSet, p=None):
+    """
+    Compute the Lp norms between the respective columns in the given data sets.
+    This means that the two datasets should have the same schema. It is
+    up to the application to ensure that the calculation is meaningless
+    :param ds1: Dataset 1
+    :param ds2: Dataset 2
+    :param p:  The order of the norm to calculate
+    :return: The calculated Lp-norm
+    """
 
     assert ds1.schema == ds2.schema, "Invalid schema for datasets"
 

src/datasets/dataset_wrapper.py

@@ -111,10 +111,16 @@ def sample_column(self):
         col_idx = np.random.choice(col_names, 1)
         return self.get_column(col_name=col_names[col_idx])
 
-    def apply_transform(self, transform: Transform) -> None:
+    def apply_column_transform(self, column_name: str, transform: Transform) -> None:
+        """
+        Apply the given transformation on the underlying dataset
+        :param column_name: The column to transform
+        :param transform: The transformation to apply
+        :return: None
+        """
 
         # get the column
-        column = self.get_column(col_name=transform.column_name)
+        column = self.get_column(col_name=column_name)
         column = transform.act(**{"data": column})
         self.ds[transform.column_name] = column
 

src/spaces/action_space.py

@@ -1,18 +1,47 @@
+"""
+ActionSpace class. This is a wrapper to the discrete
+actions in the actions.py module
+"""
+
 from gym.spaces.discrete import Discrete
 from src.spaces.actions import ActionBase
 
 
 class ActionSpace(Discrete):
+    """
+    ActionSpace class models a discrete action space of size n
+    """
 
     def __init__(self, n: int) -> None:
 
         super(ActionSpace, self).__init__(n=n)
+
+        # the list of actions the space contains
         self.actions = []
 
-    def __getitem__(self, item):
+    def __getitem__(self, item) -> ActionBase:
+        """
+        Returns the item-th action
+        :param item: The index of the action to return
+        :return: An action obeject
+        """
         return self.actions[item]
 
+    def __setitem__(self, key: int, value: ActionBase) -> None:
+        """
+        Update the key-th Action with the new value
+        :param key: The index to the action to update
+        :param value: The new action
+        :return: None
+        """
+        self.actions[key] = value
+
     def add(self, action: ActionBase) -> None:
+        """
+        Add a new action in the space
+        :param action:
+        :return:
+        """
 
         if len(self.actions) >= self.n:
             raise ValueError("Action space is saturated. You cannot add a new action")
@@ -21,11 +50,19 @@ def add(self, action: ActionBase) -> None:
         action.idx = len(self.actions)
         self.actions.append(action)
 
-    def add_may(self, *actions) -> None:
+    def add_many(self, *actions) -> None:
+        """
+        Add many actions in one go
+        :param actions: List of actions to add
+        :return: None
+        """
         for a in actions:
             self.add(action=a)
 
     def sample_and_get(self) -> ActionBase:
-
+        """
+        Sample the space and return an action to the application
+        :return: The sampled action
+        """
         action_idx = self.sample()
         return self.actions[action_idx]

src/spaces/environment.py

@@ -175,8 +175,12 @@ def apply_action(self, action: ActionBase):
         :param action: The action to apply on the environment
         :return:
         """
+
+        if action.action_type == ActionType.IDENTITY:
+            return
+
         # apply the transform of the data set
-        self.data_set.apply_transform(transform=action)
+        self.data_set.apply_column_transform(column_name=action.column_name, transform=action)
 
     def step(self, action: ActionBase) -> TimeStep:
         """
@@ -191,14 +195,16 @@ def step(self, action: ActionBase) -> TimeStep:
         `action` will be ignored.
         """
 
+        self.apply_action(action=action)
+
         # if the action is identity don't bother
         # doing anything
-        if action.action_type == ActionType.IDENTITY:
-            return TimeStep(step_type=StepType.MID, reward=0.0,
-                            observation=self.get_ds_as_tensor().float(), discount=self.gamma)
+        #if action.action_type == ActionType.IDENTITY:
+        #    return TimeStep(step_type=StepType.MID, reward=0.0,
+        #                    observation=self.get_ds_as_tensor().float(), discount=self.gamma)
 
         # apply the transform of the data set
-        self.data_set.apply_transform(transform=action)
+        #self.data_set.apply_column_transform(transform=action)
 
         # perform the action on the data set
         self.prepare_column_states()

src/tests/test_actions.py

@@ -1,15 +1,15 @@
 import unittest
 
-from src.utils.default_hierarchy import DefaultHierarchy
+from src.utils.serial_hierarchy import SerialHierarchy
 from src.spaces.actions import ActionSuppress
 
 
 class TestActions(unittest.TestCase):
 
     def test_suppress_action_creation(self):
 
-        suppress_table = {"test": DefaultHierarchy(values=["test", "tes*", "te**", "t***", "****"]),
-                          "do_not_test": DefaultHierarchy(values=["do_not_test", "do_not_tes*", "do_not_te**", "do_not_t***", "do_not_****"])}
+        suppress_table = {"test": SerialHierarchy(values=["test", "tes*", "te**", "t***", "****"]),
+                          "do_not_test": SerialHierarchy(values=["do_not_test", "do_not_tes*", "do_not_te**", "do_not_t***", "do_not_****"])}
 
         suppress_action = ActionSuppress(column_name="none", suppress_table=suppress_table)
 
@@ -19,8 +19,8 @@ def test_suppress_action_act(self):
 
         data = ["test", "do_not_test", "invalid"]
 
-        suppress_table = {"test": DefaultHierarchy(values=["test", "tes*", "te**", "t***", "****"]),
-                          "do_not_test": DefaultHierarchy(values=["do_not_test", "do_not_tes*",
+        suppress_table = {"test": SerialHierarchy(values=["test", "tes*", "te**", "t***", "****"]),
+                          "do_not_test": SerialHierarchy(values=["do_not_test", "do_not_tes*",
                                                                   "do_not_te**", "do_not_t***", "do_not_****"])}
 
         suppress_action = ActionSuppress(column_name="none", suppress_table=suppress_table)

src/tests/test_default_hierarchy.py

@@ -1,12 +1,12 @@
 import unittest
-from src.utils.default_hierarchy import DefaultHierarchy
+from src.utils.serial_hierarchy import SerialHierarchy
 
 
 class TestDefaultHierarchy(unittest.TestCase):
 
     def test_iteration(self):
         values = ["test", "tes*", "te**", "t***", "****"]
-        d_hierarchy = DefaultHierarchy(values=values)
+        d_hierarchy = SerialHierarchy(values=values)
 
         self.assertEqual(d_hierarchy.value, values[0], "Invalid hierarchy value")
         next(d_hierarchy.__iter__())

src/tests/test_environment.py

@@ -7,7 +7,7 @@
 from src.spaces.action_space import ActionSpace
 from src.spaces.actions import ActionSuppress, ActionGeneralize
 from src.exceptions.exceptions import Error
-from src.utils.default_hierarchy import DefaultHierarchy
+from src.utils.serial_hierarchy import SerialHierarchy
 from src.utils.string_distance_calculator import DistanceType
 from src.datasets.dataset_wrapper import PandasDSWrapper
 
@@ -35,7 +35,7 @@ def setUp(self) -> None:
                                            "drop_na": True,
                                            "change_col_vals": {"diagnosis": [('N', 0)]}})
 
-    @pytest.mark.skip(reason="no way of currently testing this")
+    #@pytest.mark.skip(reason="no way of currently testing this")
     def test_prepare_column_states_throw_Error(self):
         # specify the action space. We need to establish how these actions
         # are performed
@@ -47,7 +47,7 @@ def test_prepare_column_states_throw_Error(self):
         with pytest.raises(Error):
             env.prepare_column_states()
 
-    @pytest.mark.skip(reason="no way of currently testing this")
+    #@pytest.mark.skip(reason="no way of currently testing this")
     def test_prepare_column_states(self):
         # specify the action space. We need to establish how these actions
         # are performed
@@ -59,7 +59,7 @@ def test_prepare_column_states(self):
         env.initialize_text_distances(distance_type=DistanceType.COSINE)
         env.prepare_column_states()
 
-    @pytest.mark.skip(reason="no way of currently testing this")
+    #@pytest.mark.skip(reason="no way of currently testing this")
     def test_get_numeric_ds(self):
         # specify the action space. We need to establish how these actions
         # are performed
@@ -85,24 +85,24 @@ def test_apply_action(self):
         # are performed
         action_space = ActionSpace(n=1)
 
-        generalization_table = {"Mixed White/Asian": DefaultHierarchy(values=["Mixed", ]),
-                                "Chinese": DefaultHierarchy(values=["Asian", ]),
-                                "Indian": DefaultHierarchy(values=["Asian", ]),
-                                "Mixed White/Black African": DefaultHierarchy(values=["Mixed", ]),
-                                "Black African": DefaultHierarchy(values=["Black", ]),
-                                "Asian other": DefaultHierarchy(values=["Asian", ]),
-                                "Black other": DefaultHierarchy(values=["Black", ]),
-                                "Mixed White/Black Caribbean": DefaultHierarchy(values=["Mixed", ]),
-                                "Mixed other": DefaultHierarchy(values=["Mixed", ]),
-                                "Arab": DefaultHierarchy(values=["Asian", ]),
-                                "White Irish": DefaultHierarchy(values=["White", ]),
-                                "Not stated": DefaultHierarchy(values=["Not stated"]),
-                                "White Gypsy/Traveller": DefaultHierarchy(values=["White", ]),
-                                "White British": DefaultHierarchy(values=["White", ]),
-                                "Bangladeshi": DefaultHierarchy(values=["Asian", ]),
-                                "White other": DefaultHierarchy(values=["White", ]),
-                                "Black Caribbean": DefaultHierarchy(values=["Black", ]),
-                                "Pakistani": DefaultHierarchy(values=["Asian", ])}
+        generalization_table = {"Mixed White/Asian": SerialHierarchy(values=["Mixed", ]),
+                                "Chinese": SerialHierarchy(values=["Asian", ]),
+                                "Indian": SerialHierarchy(values=["Asian", ]),
+                                "Mixed White/Black African": SerialHierarchy(values=["Mixed", ]),
+                                "Black African": SerialHierarchy(values=["Black", ]),
+                                "Asian other": SerialHierarchy(values=["Asian", ]),
+                                "Black other": SerialHierarchy(values=["Black", ]),
+                                "Mixed White/Black Caribbean": SerialHierarchy(values=["Mixed", ]),
+                                "Mixed other": SerialHierarchy(values=["Mixed", ]),
+                                "Arab": SerialHierarchy(values=["Asian", ]),
+                                "White Irish": SerialHierarchy(values=["White", ]),
+                                "Not stated": SerialHierarchy(values=["Not stated"]),
+                                "White Gypsy/Traveller": SerialHierarchy(values=["White", ]),
+                                "White British": SerialHierarchy(values=["White", ]),
+                                "Bangladeshi": SerialHierarchy(values=["Asian", ]),
+                                "White other": SerialHierarchy(values=["White", ]),
+                                "Black Caribbean": SerialHierarchy(values=["Black", ]),
+                                "Pakistani": SerialHierarchy(values=["Asian", ])}
 
         action_space.add(ActionGeneralize(column_name="ethnicity", generalization_table=generalization_table))
 

src/utils/hierarchy_base.py

@@ -1,20 +1,15 @@
-import abc
-from pathlib import Path
-from typing import TypeVar
-
+"""
+HierarchyBase. A hierarchy represents as series of transformations
+that can be applied on data. For example assume that the
+data field has the value 'foo' a hierarchy of transformations then may be
+the following list ['fo*', 'f**', '***']. If this hierarchy is fully applied
+on 'foo' then 'foo' will be completely  suppressed
+"""
 
-#HierarchyBase = TypeVar("HierarchyBase")
+import abc
 
 
 class HierarchyBase(metaclass=abc.ABCMeta):
 
     def __init__(self):
-        pass
-
-    #@abc.abstractmethod
-    #def read_from(self, filename: Path) -> HierarchyBase:
-        """
-        Reads the values of the hierarchy from the file
-        :param filename: The file to read the values of the hierarchy
-        :return: None
-        """
+        pass

src/utils/default_hierarchy.py src/utils/serial_hierarchy.pysrc/utils/default_hierarchy.py renamed to src/utils/serial_hierarchy.py

@@ -1,8 +1,17 @@
+"""
+A SerialHierarchy represents a hierarchy of transformations
+that are applied one after the other
+"""
+
 from typing import List, Any
 from src.utils.hierarchy_base import HierarchyBase
 
 
-class DefaultHierarchyIterator(object):
+class SerialtHierarchyIterator(object):
+    """
+    SerialtHierarchyIterator class. Helper class to iterate over a
+    SerialHierarchy object
+    """
 
     def __init__(self, values: List):
         self.current_position = 0
@@ -27,11 +36,23 @@ def __next__(self):
         raise StopIteration
 
 
-class DefaultHierarchy(HierarchyBase):
+class SerialHierarchy(HierarchyBase):
 
+    """
+    A SerialHierarchy represents a hierarchy of transformations
+    that are applied one after the other. Applications should explicitly
+    provide the list of the ensuing transformations. For example assume that the
+    data field has the value 'foo' then values
+the following list ['fo*', 'f**', '***']
+    """
     def __init__(self, values: List) -> None:
-        super(DefaultHierarchy, self).__init__()
-        self.iterator = DefaultHierarchyIterator(values=values)
+        """
+        Constructor. Initialize the hierarchy by passing the
+        list of the ensuing transformations.
+        :param values:
+        """
+        super(SerialHierarchy, self).__init__()
+        self.iterator = SerialtHierarchyIterator(values=values)
 
     def __iter__(self):
         """
@@ -42,5 +63,8 @@ def __iter__(self):
 
     @property
     def value(self) -> Any:
+        """
+        :return: the current value the hierarchy assumes
+        """
         return self.iterator.at