Repository: HK3-Lab-Team/pytrousse

Issue:
FeatureOperation: OrdinalEncoder
 [3c iii]

Discussion:
alessiamarcolini: @lorenz-gorini actually all the modifications you commented about here are all placeholders... none of the code in feature_fix really works, here I just removed leftovers, since the Enum was gone a few PRs ago... 

Patch:
diff --git a/src/trousse/feature_fix.py b/src/trousse/feature_fix.py
index 34980f2..7512603 100644
--- a/src/trousse/feature_fix.py
+++ b/src/trousse/feature_fix.py
@@ -1,13 +1,13 @@
 import itertools
 import logging
-from typing import Tuple
+from typing import Any, Tuple
 
 import numpy as np
 import pandas as pd
-from sklearn.preprocessing import OneHotEncoder, OrdinalEncoder
+from sklearn.preprocessing import OneHotEncoder
 
-from .dataset import Dataset, FeatureOperation, copy_dataset_with_new_df
-from .feature_enum import ENCODED_COLUMN_SUFFIX, EncodingFunctions, OperationTypeEnum
+from .dataset import Dataset, copy_dataset_with_new_df
+from .feature_operations import FeatureOperation, OrdinalEncoder
 
 logger = logging.getLogger(__name__)
 
@@ -82,7 +82,7 @@ def split_continuous_column_into_bins(dataset: Dataset, col_name, bin_threshold)
     dataset.track_history(
         FeatureOperation(
             original_columns=col_name,
-            operation_type=OperationTypeEnum.BIN_SPLITTING,
+            operation_type="BIN_SPLITTING",
             encoded_values_map=bin_id_range_map,
             derived_columns=new_col_name,
         )
@@ -117,7 +117,7 @@ def combine_categorical_columns_to_one(
         Name of the new column
     """
     # Define the name of the new column containing the combination of 'column_list' values
-    new_column_name = f"{'-'.join([c for c in columns_list])}{ENCODED_COLUMN_SUFFIX}"
+    new_column_name = f"{'-'.join([c for c in columns_list])}_enc"
 
     # If the column has already been created, return the dataset
     if new_column_name in dataset.data.columns:
@@ -168,7 +168,7 @@ def combine_categorical_columns_to_one(
     dataset.track_history(
         FeatureOperation(
             original_columns=columns_list,
-            operation_type=OperationTypeEnum.FEAT_COMBOS_ENCODING,
+            operation_type="FEAT_COMBOS_ENCODING",
             encoded_values_map=new_columns_encoding_maps,
             derived_columns=new_column_name,
         )
@@ -213,9 +213,7 @@ def _one_hot_encode_column(
     except ValueError:
         logger.debug(f"No NaN values were found in column {column}")
     # Name the new columns after the categories (adding a suffix). Exclude the first which was dropped
-    new_column_names = [
-        f"{column}_{col}{ENCODED_COLUMN_SUFFIX}" for col in encoded_categories[1:]
-    ]
+    new_column_names = [f"{column}_{col}_enc" for col in encoded_categories[1:]]
     # Add the new encoded columns to the df_new
     for i, col in enumerate(new_column_names):
         df_new[col] = transformed_cols[:, i]
@@ -241,27 +239,19 @@ def _ordinal_encode_column(df, column, drop_old_column: bool = False):
     -------
 
     """
-    df_new = df.copy()
-    series = df_new[column].values
-    # Adding a new dimension for use with OrdinalEncoder (1D array to column vector shape=(5,1))
-    series = series[..., np.newaxis]
-    encoder = OrdinalEncoder()
-    encoder_fitted = encoder.fit(series)  # Encoder object (for reverse transformation)
-    series_enc = encoder_fitted.transform(series)
-    new_column = f"{column}{ENCODED_COLUMN_SUFFIX}"
-    # Convert the encoded columns to Integer type (this pandas Dtype is for handling NaN values)
-    df_new[new_column] = series_enc
-    df_new[new_column] = df_new[new_column].astype(pd.Int16Dtype())
 
-    if drop_old_column:
-        df_new = df_new.drop(column, axis=1)
-    return df_new, encoder_fitted, [new_column]
+    dataset = Dataset(df_object=df)
+    derived_column = f"{column}_enc"
+    ordinal_encoder = OrdinalEncoder(columns=[column], derived_columns=[derived_column])
+
+    encoded_dataset = ordinal_encoder(dataset)
+    return encoded_dataset.data, ordinal_encoder.encoder, [derived_column]
 
 
 def encode_single_categorical_column(
     dataset: Dataset,
     col_name: str,
-    encoding: EncodingFunctions = EncodingFunctions.ORDINAL,
+    encoding: Any = "EncodingFunctions.ORDINAL",
     drop_one_new_column: bool = True,
     drop_old_column: bool = False,
     force: bool = False,
@@ -321,11 +311,11 @@ def encode_single_categorical_column(
         df_to_encode.loc[:, col_name] = df_to_encode[col_name].astype(str).str.title()
 
     # Encoding using the selected function
-    if encoding == EncodingFunctions.ORDINAL:
+    if encoding == "ORDINAL":
         df_encoded, encoder, new_columns = _ordinal_encode_column(
             df_to_encode, column=col_name, drop_old_column=drop_old_column
         )
-    elif encoding == EncodingFunctions.ONEHOT:
+    elif encoding == "ONEHOT":
         df_encoded, encoder, new_columns = _one_hot_encode_column(
             df_to_encode,
             column=col_name,
@@ -335,7 +325,7 @@ def encode_single_categorical_column(
     else:
         logging.error(
             f"No valid encoding_func argument. Possible "
-            f"values are: {[e.name for e in EncodingFunctions]}"
+            f"values are: {[e.name for e in ['ORDINAL', 'ONEHOT']]}"
         )
         return None
 
@@ -353,7 +343,7 @@ def encode_single_categorical_column(
     dataset_encoded.track_history(
         FeatureOperation(
             original_columns=col_name,
-            operation_type=OperationTypeEnum.CATEGORICAL_ENCODING,
+            operation_type="CATEGORICAL_ENCODING",
             encoder=encoder,
             encoded_values_map=encoded_values_map,
             derived_columns=tuple(new_columns),
@@ -366,7 +356,7 @@ def encode_single_categorical_column(
 def encode_multi_categorical_columns(
     dataset: Dataset,
     columns: Tuple = None,
-    encoding: EncodingFunctions = EncodingFunctions.ORDINAL,
+    encoding: Any = "ORDINAL",
     drop_one_new_column: bool = True,
     drop_old_column: bool = False,
 ):
diff --git a/src/trousse/feature_operations.py b/src/trousse/feature_operations.py
index e19380c..949897d 100644
--- a/src/trousse/feature_operations.py
+++ b/src/trousse/feature_operations.py
@@ -5,6 +5,7 @@ from typing import Any, List, Mapping
 
 from .dataset import Dataset
 from .util import is_sequence_and_not_str
+import sklearn.preprocessing as sk_preproc
 
 
 class FeatureOperation(ABC):
@@ -456,3 +457,99 @@ class ReplaceStrings(ReplaceSubstrings):
             f"\n\treplacement_map={self.replacement_map},"
             f"\n\tderived_columns={self.derived_columns},\n)"
         )
+
+
+class OrdinalEncoder(FeatureOperation):
+    """Encode ``columns`` with a ordinal encoder.
+
+    By default, the column is encoded in place. To store the result of the encoding in
+    another column, ``derived_columns`` parameter has to be set with
+    the name of the corresponding column name.
+
+    Parameters
+    ----------
+    columns : List[str]
+        Name of the column to encode. It must be a single-element list.
+    derived_columns : List[str], optional
+        Name of the column where to store the encoding result. Default is None,
+        meaning that the original column is encoded in place. If not None, it
+        must be a single-element list.
+
+    Returns
+    -------
+    Dataset
+        The new Dataset with the column encoded.
+
+    Raises
+    ------
+    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__(self, columns: List[str], derived_columns: List[str] = None) -> None:
+        self._validate_single_element_columns(columns)
+        self._validate_single_element_derived_columns(derived_columns)
+
+        self.columns = columns
+        self.derived_columns = derived_columns
+        self._encoder = sk_preproc.OrdinalEncoder()
+
+    @property
+    def encoder(self) -> sk_preproc.OrdinalEncoder:
+        return self._encoder
+
+    def _apply(self, dataset: Dataset) -> Dataset:
+        """Apply OrdinalEncoder operation on a new Dataset instance and return it.
+
+        Parameters
+        ----------
+        dataset : Dataset
+            The dataset to apply the operation on
+
+        Returns
+        -------
+        Dataset
+            New Dataset instance with the operation applied on
+        """
+        dataset = copy.deepcopy(dataset)
+        data = dataset.data[[self.columns[0]]]
+
+        series_enc = self._encoder.fit_transform(data).astype("int64")
+
+        if self.derived_columns is not None:
+            dataset.data[self.derived_columns[0]] = series_enc
+        else:
+            dataset.data[self.columns[0]] = series_enc
+
+        return dataset
+
+    def __eq__(self, other: Any) -> bool:
+        """Return True if ``other`` is a OrdinalEncoder instance and it has the same fields value.
+
+        Parameters
+        ----------
+        other : Any
+            The instance to compare
+
+        Returns
+        -------
+        bool
+            True if ``other`` is a OrdinalEncoder instance and it has the same fields
+            value, False otherwise
+        """
+        if not isinstance(other, OrdinalEncoder):
+            return False
+        if (
+            self.columns == other.columns
+            and self.derived_columns == other.derived_columns
+        ):
+            return True
+
+        return False
+
+    def is_similar(self, other: FeatureOperation):
+        raise NotImplementedError


Test patch:
diff --git a/tests/expectations/csv/generic-ordinal-encoded-col3-col3_enc.csv b/tests/expectations/csv/generic-ordinal-encoded-col3-col3_enc.csv
new file mode 100644
index 0000000..4c2e861
--- /dev/null
+++ b/tests/expectations/csv/generic-ordinal-encoded-col3-col3_enc.csv
@@ -0,0 +1,5 @@
+col0,col1,col2,col3,col3_enc
+a,b,e,abc,0
+a,b,c,abc,0
+d,b,c,abr,1
+a,b,c,abc,0
diff --git a/tests/expectations/csv/generic-ordinal-encoded-col3-col4.csv b/tests/expectations/csv/generic-ordinal-encoded-col3-col4.csv
new file mode 100644
index 0000000..05bb794
--- /dev/null
+++ b/tests/expectations/csv/generic-ordinal-encoded-col3-col4.csv
@@ -0,0 +1,5 @@
+col0,col1,col2,col3,col4
+a,b,e,abc,0
+a,b,c,abc,0
+d,b,c,abr,1
+a,b,c,abc,0
diff --git a/tests/expectations/csv/generic-ordinal-encoded-col3-inplace.csv b/tests/expectations/csv/generic-ordinal-encoded-col3-inplace.csv
new file mode 100644
index 0000000..24fc159
--- /dev/null
+++ b/tests/expectations/csv/generic-ordinal-encoded-col3-inplace.csv
@@ -0,0 +1,5 @@
+col0,col1,col2,col3
+a,b,e,0
+a,b,c,0
+d,b,c,1
+a,b,c,0
diff --git a/tests/integration/test_feature_fix.py b/tests/integration/test_feature_fix.py
new file mode 100644
index 0000000..3d17ef6
--- /dev/null
+++ b/tests/integration/test_feature_fix.py
@@ -0,0 +1,28 @@
+import pandas as pd
+import pytest
+
+import trousse.feature_fix as ffx
+from trousse.dataset import Dataset
+from ..fixtures import CSV
+from ..util import load_expectation
+
+
+@pytest.mark.parametrize(
+    "csv, column, derived_column, expected_csv",
+    (
+        (
+            CSV.generic,
+            "col3",
+            ["col3_enc"],
+            "csv/generic-ordinal-encoded-col3-col3_enc",
+        ),
+    ),
+)
+def test_ordinal_encode_column(csv, column, derived_column, expected_csv):
+    dataset = Dataset(data_file=csv)
+    expected_df = load_expectation(expected_csv, type_="csv")
+
+    encoded_df, _, new_cols = ffx._ordinal_encode_column(dataset.data, column, False)
+
+    pd.testing.assert_frame_equal(encoded_df, expected_df)
+    assert derived_column == new_cols
diff --git a/tests/integration/test_feature_operations.py b/tests/integration/test_feature_operations.py
index 4d85c3b..691baee 100644
--- a/tests/integration/test_feature_operations.py
+++ b/tests/integration/test_feature_operations.py
@@ -89,3 +89,32 @@ def test_replace_substrings(csv, columns, derived_columns, expected_csv):
     replaced_dataset = replace_substrings(dataset)
 
     pd.testing.assert_frame_equal(replaced_dataset.data, expected_df)
+
+
+@pytest.mark.parametrize(
+    "csv, columns, derived_columns, expected_csv",
+    (
+        (
+            CSV.generic,
+            ["col3"],
+            None,
+            "csv/generic-ordinal-encoded-col3-inplace",
+        ),
+        (
+            CSV.generic,
+            ["col3"],
+            ["col4"],
+            "csv/generic-ordinal-encoded-col3-col4",
+        ),
+    ),
+)
+def test_ordinal_encoder(csv, columns, derived_columns, expected_csv):
+    dataset = Dataset(data_file=csv)
+    expected_df = load_expectation(expected_csv, type_="csv")
+    ordinal_encoder = fop.OrdinalEncoder(
+        columns=columns, derived_columns=derived_columns
+    )
+
+    encoded_dataset = ordinal_encoder(dataset)
+
+    pd.testing.assert_frame_equal(encoded_dataset.data, expected_df)
diff --git a/tests/unit/test_feature_operations.py b/tests/unit/test_feature_operations.py
index 2a85462..61cdca6 100644
--- a/tests/unit/test_feature_operations.py
+++ b/tests/unit/test_feature_operations.py
@@ -5,6 +5,8 @@ from trousse import feature_operations as fop
 from trousse.dataset import Dataset
 
 from ..dataset_util import DataFrameMock
+import sklearn.preprocessing as sk_preproc
+
 from ..unitutil import ANY, function_mock, initializer_mock, method_mock
 
 
@@ -577,3 +579,150 @@ class DescribeReplaceSubstrings:
             "ReplaceSubstrings(\n\tcolumns=['exam_num_col_0'],\n\treplacement_map="
             "{'a': 'b', 'c': 'd'},\n\tderived_columns=['replaced_exam_num_col_0'],\n)"
         )
+
+
+class DescribeOrdinalEncoder:
+    def it_construct_from_args(self, request):
+        _init_ = initializer_mock(request, fop.OrdinalEncoder)
+
+        ordinal_encoder = fop.OrdinalEncoder(columns=["col0"], derived_columns=["col1"])
+
+        _init_.assert_called_once_with(
+            ANY,
+            columns=["col0"],
+            derived_columns=["col1"],
+        )
+        assert isinstance(ordinal_encoder, fop.OrdinalEncoder)
+
+    def and_it_validates_its_arguments(self, request):
+        validate_columns_ = method_mock(
+            request, fop.OrdinalEncoder, "_validate_single_element_columns"
+        )
+        validate_derived_columns_ = method_mock(
+            request, fop.OrdinalEncoder, "_validate_single_element_derived_columns"
+        )
+
+        ordinal_encoder = fop.OrdinalEncoder(columns=["col0"], derived_columns=["col1"])
+
+        validate_columns_.assert_called_once_with(ordinal_encoder, ["col0"])
+        validate_derived_columns_.assert_called_once_with(ordinal_encoder, ["col1"])
+
+    def it_knows_its_encoder(self):
+        ordinal_encoder = fop.OrdinalEncoder(columns=["col0"], derived_columns=["col1"])
+
+        encoder_attr = ordinal_encoder.encoder
+
+        assert isinstance(encoder_attr, sk_preproc.OrdinalEncoder)
+
+    @pytest.mark.parametrize(
+        "columns, derived_columns, expected_new_columns",
+        [
+            (
+                ["exam_str_col_0"],
+                ["col1"],
+                ["col1"],
+            ),
+            (
+                ["exam_str_col_0"],
+                None,
+                [],
+            ),
+        ],
+    )
+    def it_can_apply_ordinal_encoder(
+        self,
+        request,
+        columns,
+        derived_columns,
+        expected_new_columns,
+    ):
+        df = DataFrameMock.df_generic(sample_size=100)
+        get_df_from_csv_ = function_mock(request, "trousse.dataset.get_df_from_csv")
+        get_df_from_csv_.return_value = df
+        dataset = Dataset(data_file="fake/path0")
+        sk_fit_transform_ = method_mock(
+            request, sk_preproc.OrdinalEncoder, "fit_transform"
+        )
+        sk_fit_transform_.return_value = pd.Series(range(100))
+        ordinal_encoder = fop.OrdinalEncoder(
+            columns=columns,
+            derived_columns=derived_columns,
+        )
+
+        encoded_dataset = ordinal_encoder._apply(dataset)
+
+        assert encoded_dataset is not None
+        assert encoded_dataset is not dataset
+        assert isinstance(encoded_dataset, Dataset)
+        for col in expected_new_columns:
+            assert col in encoded_dataset.data.columns
+        get_df_from_csv_.assert_called_once_with("fake/path0")
+        assert len(sk_fit_transform_.call_args_list) == len(columns)
+        pd.testing.assert_frame_equal(
+            sk_fit_transform_.call_args_list[0][0][1], df[[columns[0]]]
+        )
+
+    def it_can_encode_with_template_call(self, request):
+        _apply_ = method_mock(request, fop.OrdinalEncoder, "_apply")
+        track_history_ = method_mock(request, Dataset, "track_history")
+        df = DataFrameMock.df_generic(sample_size=100)
+        get_df_from_csv_ = function_mock(request, "trousse.dataset.get_df_from_csv")
+        get_df_from_csv_.return_value = df
+        dataset_in = Dataset(data_file="fake/path0")
+        dataset_out = Dataset(data_file="fake/path0")
+        _apply_.return_value = dataset_out
+        ordinal_encoder = fop.OrdinalEncoder(
+            columns=["exam_num_col_0"],
+            derived_columns=["exam_str_col_0"],
+        )
+
+        replaced_dataset = ordinal_encoder(dataset_in)
+
+        _apply_.assert_called_once_with(ordinal_encoder, dataset_in)
+        track_history_.assert_called_once_with(replaced_dataset, ordinal_encoder)
+        assert replaced_dataset is dataset_out
+
+    @pytest.mark.parametrize(
+        "other, expected_equal",
+        [
+            (
+                fop.OrdinalEncoder(
+                    columns=["exam_num_col_0"],
+                    derived_columns=["encoded_exam_num_col_0"],
+                ),
+                True,
+            ),
+            (
+                fop.OrdinalEncoder(
+                    columns=["exam_num_col_1"],
+                    derived_columns=["encoded_exam_num_col_0"],
+                ),
+                False,
+            ),
+            (
+                fop.OrdinalEncoder(
+                    columns=["exam_num_col_0"],
+                    derived_columns=["encoded_exam_num_col_1"],
+                ),
+                False,
+            ),
+            (
+                fop.OrdinalEncoder(
+                    columns=["exam_num_col_1"],
+                    derived_columns=["encoded_exam_num_col_1"],
+                ),
+                False,
+            ),
+            (dict(), False),
+        ],
+    )
+    def it_knows_if_equal(self, other, expected_equal):
+        feat_op = fop.OrdinalEncoder(
+            columns=["exam_num_col_0"],
+            derived_columns=["encoded_exam_num_col_0"],
+        )
+
+        equal = feat_op == other
+
+        assert type(equal) == bool
+        assert equal == expected_equal