Repository: HK3-Lab-Team/pytrousse

Issue:
implement OperationsList, class to represent a list of FeatureOperations

Discussion:

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
new file mode 100644
index 0000000..f2a56be
--- /dev/null
+++ b/.pre-commit-config.yaml
@@ -0,0 +1,11 @@
+  
+repos:
+-   repo: https://github.com/ambv/black
+    rev: stable
+    hooks:
+    - id: black
+      language_version: python3.7
+-   repo: https://gitlab.com/pycqa/flake8
+    rev: 3.7.9
+    hooks:
+    - id: flake8
\ No newline at end of file
diff --git a/requirements-dev.txt b/requirements-dev.txt
index e8beb24..5064bb2 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -4,4 +4,5 @@ isort
 pytest
 pytest-cov
 coverage
-ipdb
\ No newline at end of file
+ipdb
+pre-commit
\ No newline at end of file
diff --git a/src/trousse/feature_operations.py b/src/trousse/feature_operations.py
index 3969ec0..131aa73 100644
--- a/src/trousse/feature_operations.py
+++ b/src/trousse/feature_operations.py
@@ -3,10 +3,12 @@ try:
 except ImportError:
     from typing_extensions import Protocol, runtime_checkable
 
+import collections
 from abc import abstractmethod
-from typing import Any, List
+from typing import Any, List, Union
 
 from .dataset import Dataset
+from .util import is_sequence_and_not_str
 
 
 @runtime_checkable
@@ -25,6 +27,135 @@ class FeatureOperation(Protocol):
         raise NotImplementedError
 
 
+class _OperationsList:
+    def __init__(self):
+        self._operations_list = []
+        self._operations_by_column = collections.defaultdict(list)
+
+    def derived_columns_from_col(self, column: str) -> List[str]:
+        """Return name of the columns created by FeatureOperations applied on ``column``
+
+        Parameters
+        ----------
+        column : str
+            The column on which the FeatureOperation has been applied on.
+
+        Returns
+        -------
+        List[str]
+            Name of the columns created by FeatureOperations applied on ``column``.
+        """
+        derived_columns = []
+        operations = self[column]
+
+        for operation in operations:
+            if column in operation.columns and operation.derived_columns is not None:
+                derived_columns.extend(operation.derived_columns)
+
+        return derived_columns
+
+    def original_columns_from_derived_column(self, derived_column: str) -> List[str]:
+        """Return the name of the columns from which ``derived_column`` is generated from.
+
+        Parameters
+        ----------
+        derived_column : str
+            The column that has been generated
+
+        Returns
+        -------
+        List[str]
+            Name of the columns from which ``derived_column`` has been generated from.
+        """
+        operations = self._operations_from_derived_column(derived_column)
+
+        if len(operations) > 1:
+            raise RuntimeError(
+                "Multiple FeatureOperation found that generated column "
+                f"{derived_column}... the pipeline is compromised"
+            )
+        if len(operations) == 0:
+            raise RuntimeError(
+                "No FeatureOperation found that generated column "
+                f"{derived_column}... the pipeline is compromised"
+            )
+
+        return operations[0].columns
+
+    def _operations_from_derived_column(
+        self, derived_column: str
+    ) -> List[FeatureOperation]:
+        """Return the FeatureOperations that generated ``derived_column``
+
+        Parameters
+        ----------
+        derived_column : str
+            The column that has been generated
+
+        Returns
+        -------
+        List[FeatureOperation]
+            FeatureOperations that generated ``derived_column``
+        """
+        return list(
+            filter(
+                lambda op: op.derived_columns is not None
+                and derived_column in op.derived_columns,
+                self[derived_column],
+            )
+        )
+
+    def __iadd__(self, feat_op: FeatureOperation):
+        self._operations_list.append(feat_op)
+
+        derived_columns = (
+            feat_op.derived_columns if feat_op.derived_columns is not None else []
+        )
+
+        for column in feat_op.columns + derived_columns:
+            self._operations_by_column[column].append(feat_op)
+
+        return self
+
+    def __iter__(self):
+        for operation in self._operations_list:
+            yield operation
+
+    def __getitem__(
+        self, label: Union[int, str]
+    ) -> Union[FeatureOperation, List[FeatureOperation]]:
+        """Retrieve FeatureOperation element.
+
+        Parameters
+        ----------
+        label : Union[int, str]
+            Label used to retrieve the element. If int: get the label-th FeatureOperation.
+            If str: ``label`` will be treated as the name of a column and it will get all
+            the FeatureOperation associated with that column.
+
+        Returns
+        -------
+        Union[FeatureOperation, List[FeatureOperation]]
+            Requested FeatureOperation(s)
+
+        Raises
+        ------
+        TypeError
+            If ``label`` is not either an integer or a string.
+        """
+        if isinstance(label, int):
+            return self._operations_list[label]
+        elif isinstance(label, str):
+            return self._operations_by_column[label]
+        else:
+            raise TypeError(
+                f"Cannot get FeatureOperation with a label of type {type(label).__name__}"
+            )
+
+    def __len__(self):
+        return len(self._operations_list)
+
+
 class FillNA(FeatureOperation):
     """Fill NaN values ``columns`` (single-element list) column with value ``value``.
 
@@ -52,6 +183,10 @@ class FillNA(FeatureOperation):
     ------
     ValueError
         If ``columns`` or ``derived_columns`` are not a single-element list.
+    TypeError
+            If ``columns`` is not a list
+    TypeError
+        If ``derived_columns`` is not None and it is not a list
     """
 
     def __init__(
@@ -60,12 +195,21 @@ class FillNA(FeatureOperation):
         value: Any,
         derived_columns: List[str] = None,
     ):
+        if not is_sequence_and_not_str(columns):
+            raise TypeError(
+                f"columns parameter must be a list, found {type(columns).__name__}"
+            )
         if len(columns) != 1:
             raise ValueError(f"Length of columns must be 1, found {len(columns)}")
-        if derived_columns is not None and len(derived_columns) != 1:
-            raise ValueError(
-                f"Length of derived_columns must be 1, found {len(derived_columns)}"
-            )
+        if derived_columns is not None:
+            if not is_sequence_and_not_str(derived_columns):
+                raise TypeError(
+                    f"derived_columns parameter must be a list, found {type(derived_columns).__name__}"
+                )
+            if len(derived_columns) != 1:
+                raise ValueError(
+                    f"Length of derived_columns must be 1, found {len(derived_columns)}"
+                )
 
         self.columns = columns
         self.derived_columns = derived_columns
diff --git a/src/trousse/util.py b/src/trousse/util.py
index 52dd817..01b20b9 100644
--- a/src/trousse/util.py
+++ b/src/trousse/util.py
@@ -1,6 +1,22 @@
 import functools
-from typing import Any, Callable
+from typing import Any, Callable, Sequence
 
 
 def lazy_property(f: Callable[..., Any]):
     return property(functools.lru_cache()(f))
+
+
+def is_sequence_and_not_str(obj: Any) -> bool:
+    """Return True if ``obj`` is a sequence object but not a string.
+
+    Parameters
+    ----------
+    obj : Any
+        Object to check
+
+    Returns
+    -------
+    bool
+        True if ``obj`` is a sequence object but not a string
+    """
+    return not isinstance(obj, str) and isinstance(obj, Sequence)


Test patch:
diff --git a/tests/unit/test_dataset.py b/tests/unit/test_dataset.py
index 5eb64ab..e2aa8c8 100644
--- a/tests/unit/test_dataset.py
+++ b/tests/unit/test_dataset.py
@@ -14,7 +14,7 @@ class DescribeDataset:
         "metadata_cols",
         [
             (("metadata_num_col")),
-            (("metadata_num_col, string_col")),
+            (("metadata_num_col", "string_col")),
             (()),
         ],
     )
diff --git a/tests/unit/test_feature_operations.py b/tests/unit/test_feature_operations.py
index 471db77..81c72b2 100644
--- a/tests/unit/test_feature_operations.py
+++ b/tests/unit/test_feature_operations.py
@@ -1,8 +1,10 @@
+from collections.abc import Iterable
+
 import pytest
 from trousse import feature_operations as fop
 from trousse.dataset import Dataset
 
-from ..unitutil import ANY, initializer_mock, method_mock
+from ..unitutil import ANY, function_mock, initializer_mock, method_mock
 
 
 class DescribeFillNa:
@@ -24,8 +26,10 @@ class DescribeFillNa:
         ],
     )
     def but_it_raises_valueerror_with_columns_length_different_than_1(
-        self, columns, expected_length
+        self, columns, expected_length, is_sequence_and_not_str_
     ):
+        is_sequence_and_not_str_.return_value = True
+
         with pytest.raises(ValueError) as err:
             fop.FillNA(columns=columns, value=0)
 
@@ -40,8 +44,10 @@ class DescribeFillNa:
         ],
     )
     def but_it_raises_valueerror_with_derived_columns_length_different_than_1(
-        self, derived_columns, expected_length
+        self, derived_columns, expected_length, is_sequence_and_not_str_
     ):
+        is_sequence_and_not_str_.return_value = True
+
         with pytest.raises(ValueError) as err:
             fop.FillNA(columns=["nan"], derived_columns=derived_columns, value=0)
 
@@ -50,6 +56,41 @@ class DescribeFillNa:
             err.value
         )
 
+    @pytest.mark.parametrize(
+        "columns, expected_type",
+        [("nan", "str"), (dict(), "dict"), (set(), "set")],
+    )
+    def but_it_raises_typeerror_with_columns_not_list(
+        self, columns, expected_type, is_sequence_and_not_str_
+    ):
+        is_sequence_and_not_str_.return_value = False
+
+        with pytest.raises(TypeError) as err:
+            fop.FillNA(columns=columns, value=0)
+
+        assert isinstance(err.value, TypeError)
+        assert f"columns parameter must be a list, found {expected_type}" == str(
+            err.value
+        )
+
+    @pytest.mark.parametrize(
+        "derived_columns, expected_type",
+        [("nan", "str"), (dict(), "dict"), (set(), "set")],
+    )
+    def but_it_raises_typeerror_with_derived_columns_not_list(
+        self, derived_columns, expected_type, is_sequence_and_not_str_
+    ):
+        is_sequence_and_not_str_.side_effect = [True, False]
+
+        with pytest.raises(TypeError) as err:
+            fop.FillNA(columns=["nan"], derived_columns=derived_columns, value=0)
+
+        assert isinstance(err.value, TypeError)
+        assert (
+            f"derived_columns parameter must be a list, found {expected_type}"
+            == str(err.value)
+        )
+
     def it_calls_fillna(self, request):
         initializer_mock(request, Dataset)
         fillna_ = method_mock(request, Dataset, "fillna")
@@ -72,3 +113,236 @@ class DescribeFillNa:
             inplace=False,
         )
         assert isinstance(filled_dataset, Dataset)
+
+    # ====================
+    #      FIXTURES
+    # ====================
+
+    @pytest.fixture
+    def is_sequence_and_not_str_(self, request):
+        return function_mock(
+            request, "trousse.feature_operations.is_sequence_and_not_str"
+        )
+
+
+class DescribeOperationsList:
+    def it_can_construct_itself(self, request):
+        _init_ = initializer_mock(request, fop._OperationsList)
+
+        operations_list = fop._OperationsList()
+
+        _init_.assert_called_once_with(ANY)
+        assert isinstance(operations_list, fop._OperationsList)
+
+    def it_can_iadd_first_featop(self, request, fillna_col0_col1):
+        op_list = fop._OperationsList()
+
+        op_list += fillna_col0_col1
+
+        assert op_list._operations_list == [fillna_col0_col1]
+        for column in ["col0", "col1"]:
+            assert op_list._operations_by_column[column] == [fillna_col0_col1]
+
+    def it_can_iadd_next_featop(self, request, fillna_col0_col1, fillna_col4_none):
+        op_list = fop._OperationsList()
+        op_list._operations_list = [fillna_col0_col1]
+        for column in ["col0", "col1"]:
+            op_list._operations_by_column[column] = [fillna_col0_col1]
+
+        op_list += fillna_col4_none
+
+        assert op_list._operations_list == [fillna_col0_col1, fillna_col4_none]
+        assert op_list._operations_by_column["col0"] == [fillna_col0_col1]
+        assert op_list._operations_by_column["col1"] == [
+            fillna_col0_col1,
+        ]
+        assert op_list._operations_by_column["col4"] == [fillna_col4_none]
+
+    def it_can_getitem_from_int(self, fillna_col0_col1, fillna_col1_col4):
+        op_list = fop._OperationsList()
+        op_list._operations_list = [fillna_col0_col1, fillna_col1_col4]
+        op_list._operations_by_column["col0"] = [fillna_col0_col1]
+        op_list._operations_by_column["col1"] = [fillna_col0_col1, fillna_col1_col4]
+        op_list._operations_by_column["col4"] = [fillna_col1_col4]
+
+        feat_op0_ = op_list[0]
+        feat_op1_ = op_list[1]
+
+        assert isinstance(feat_op0_, fop.FillNA)
+        assert isinstance(feat_op1_, fop.FillNA)
+        assert feat_op0_ == fillna_col0_col1
+        assert feat_op1_ == fillna_col1_col4
+
+    def it_can_getitem_from_str(self, fillna_col0_col1, fillna_col1_col4):
+        op_list = fop._OperationsList()
+        op_list._operations_list = [fillna_col0_col1, fillna_col1_col4]
+        op_list._operations_by_column["col0"] = [fillna_col0_col1]
+        op_list._operations_by_column["col1"] = [fillna_col0_col1, fillna_col1_col4]
+        op_list._operations_by_column["col4"] = [fillna_col1_col4]
+
+        feat_op_col0 = op_list["col0"]
+        feat_op_col1 = op_list["col1"]
+
+        assert isinstance(feat_op_col0, list)
+        assert isinstance(feat_op_col1, list)
+        assert feat_op_col0 == [fillna_col0_col1]
+        assert feat_op_col1 == [fillna_col0_col1, fillna_col1_col4]
+
+    def but_it_raisestypeerror_with_wrong_type(self):
+        op_list = fop._OperationsList()
+
+        with pytest.raises(TypeError) as err:
+            op_list[{"wrong"}]
+
+        assert isinstance(err.value, TypeError)
+        assert "Cannot get FeatureOperation with a label of type set" == str(err.value)
+
+    @pytest.mark.parametrize(
+        "column, getitem_return_value, expected_derived_columns",
+        [
+            (
+                "col0",
+                [fop.FillNA(columns=["col0"], derived_columns=["col1"], value=0)],
+                ["col1"],
+            ),
+            (
+                "col1",
+                [
+                    fop.FillNA(columns=["col0"], derived_columns=["col1"], value=0),
+                    fop.FillNA(columns=["col1"], derived_columns=["col4"], value=0),
+                    fop.FillNA(columns=["col1"], derived_columns=["col2"], value=0),
+                ],
+                ["col4", "col2"],
+            ),
+            (
+                "col4",
+                [
+                    fop.FillNA(columns=["col1"], derived_columns=["col1"], value=0),
+                    fop.FillNA(columns=["col4"], derived_columns=None, value=0),
+                ],
+                [],
+            ),
+        ],
+    )
+    def it_can_get_derived_columns_from_col(
+        self, request, column, getitem_return_value, expected_derived_columns
+    ):
+        op_list = fop._OperationsList()
+        getitem_ = method_mock(request, fop._OperationsList, "__getitem__")
+        getitem_.return_value = getitem_return_value
+
+        derived_columns = op_list.derived_columns_from_col(column)
+
+        assert type(derived_columns) == list
+        assert derived_columns == expected_derived_columns
+
+    def it_can_get_operations_from_derived_column(self, request):
+        op_list = fop._OperationsList()
+        getitem_ = method_mock(request, fop._OperationsList, "__getitem__")
+        fop0 = fop.FillNA(columns=["col4"], derived_columns=["col1"], value=0)
+        fop1 = fop.FillNA(columns=["col1"], derived_columns=["col4"], value=0)
+        fop2 = fop.FillNA(columns=["col4"], derived_columns=None, value=0)
+        getitem_.return_value = [fop0, fop1, fop2]
+
+        operations = op_list._operations_from_derived_column("col4")
+
+        assert type(operations) == list
+        assert operations == [fop1]
+
+    def it_can_get_original_columns_from_derived_column(self, request):
+        op_list = fop._OperationsList()
+        _operations_from_derived_column_ = method_mock(
+            request, fop._OperationsList, "_operations_from_derived_column"
+        )
+        _operations_from_derived_column_.return_value = [
+            fop.FillNA(columns=["col0"], derived_columns=["col1"], value=0)
+        ]
+
+        original_columns = op_list.original_columns_from_derived_column("col1")
+
+        assert type(original_columns) == list
+        assert original_columns == ["col0"]
+
+    def but_it_raises_runtimeerror_with_multiple_operations_found(self, request):
+        op_list = fop._OperationsList()
+        _operations_from_derived_column_ = method_mock(
+            request, fop._OperationsList, "_operations_from_derived_column"
+        )
+        _operations_from_derived_column_.return_value = [
+            fop.FillNA(columns=["col0"], derived_columns=["col1"], value=0),
+            fop.FillNA(columns=["col2"], derived_columns=["col1"], value=0),
+        ]
+
+        with pytest.raises(RuntimeError) as err:
+            op_list.original_columns_from_derived_column("col1")
+
+        assert isinstance(err.value, RuntimeError)
+        assert (
+            "Multiple FeatureOperation found that generated column "
+            "col1... the pipeline is compromised"
+        ) == str(err.value)
+
+    def but_it_raises_runtimeerror_with_zero_operations_found(self, request):
+        op_list = fop._OperationsList()
+        _operations_from_derived_column_ = method_mock(
+            request, fop._OperationsList, "_operations_from_derived_column"
+        )
+        _operations_from_derived_column_.return_value = []
+
+        with pytest.raises(RuntimeError) as err:
+            op_list.original_columns_from_derived_column("col1")
+
+        assert isinstance(err.value, RuntimeError)
+        assert (
+            "No FeatureOperation found that generated column "
+            "col1... the pipeline is compromised"
+        ) == str(err.value)
+
+    def it_knows_its_len(self, fillna_col0_col1, fillna_col1_col4):
+        op_list = fop._OperationsList()
+        op_list._operations_list = [fillna_col0_col1, fillna_col1_col4]
+
+        len_ = len(op_list)
+
+        assert type(len_) == int
+        assert len_ == 2
+
+    def it_can_be_iterated_over(
+        self, fillna_col0_col1, fillna_col1_col4, fillna_col1_col2
+    ):
+        op_list = fop._OperationsList()
+        op_list._operations_list = [
+            fillna_col0_col1,
+            fillna_col1_col4,
+            fillna_col1_col2,
+        ]
+        operations = []
+        for operation in op_list:
+            operations.append(operation)
+
+        assert isinstance(op_list, Iterable)
+        assert operations == [
+            fillna_col0_col1,
+            fillna_col1_col4,
+            fillna_col1_col2,
+        ]
+
+    # ====================
+    #      FIXTURES
+    # ====================
+
+    @pytest.fixture
+    def fillna_col0_col1(self, request):
+        return fop.FillNA(columns=["col0"], derived_columns=["col1"], value=0)
+
+    @pytest.fixture
+    def fillna_col1_col4(self, request):
+        return fop.FillNA(columns=["col1"], derived_columns=["col4"], value=0)
+
+    @pytest.fixture
+    def fillna_col4_none(self, request):
+        return fop.FillNA(columns=["col4"], derived_columns=None, value=0)
+
+    @pytest.fixture
+    def fillna_col1_col2(self, request):
+        return fop.FillNA(columns=["col1"], derived_columns=["col2"], value=0)
diff --git a/tests/unit/test_util.py b/tests/unit/test_util.py
new file mode 100644
index 0000000..d7c68b4
--- /dev/null
+++ b/tests/unit/test_util.py
@@ -0,0 +1,13 @@
+import pytest
+from trousse import util
+
+
+@pytest.mark.parametrize(
+    "obj, expected_result",
+    (("nan", False), (dict(), False), (set(), False), ([1, 2, 3], True), ([], True)),
+)
+def test_is_sequence_and_not_str(obj, expected_result):
+    result = util.is_sequence_and_not_str(obj)
+
+    assert isinstance(result, bool)
+    assert result == expected_result