Add deprecation path for the hierachical schema specification in sklearn

Author: popescu-v

khiops/sklearn/dataset.py

@@ -10,6 +10,7 @@
 import warnings
 from abc import ABC, abstractmethod
 from collections.abc import Iterable, Mapping, Sequence
+from itertools import cycle, islice
 
 import numpy as np
 import pandas as pd
@@ -21,7 +22,12 @@
 import khiops.core as kh
 import khiops.core.internals.filesystems as fs
 from khiops.core.dictionary import VariableBlock
-from khiops.core.internals.common import is_dict_like, is_list_like, type_error_message
+from khiops.core.internals.common import (
+    deprecation_message,
+    is_dict_like,
+    is_list_like,
+    type_error_message,
+)
 
 # Disable PEP8 variable names because of scikit-learn X,y conventions
 # To capture invalid-names other than X,y run:
@@ -171,6 +177,48 @@ def _check_multitable_spec(ds_spec):
             )
 
 
+def _table_name_of_path(table_path):
+    return table_path.split("/")[-1]
+
+
+def _upgrade_mapping_spec(X):
+    assert is_dict_like(X)
+    new_X = {}
+    new_X["additional_data_tables"] = {}
+    for table_name, table_data in X["tables"].items():
+        table_df, table_key = table_data
+        if not is_list_like(table_key):
+            table_key = [table_key]
+        if table_name == X["main_table"]:
+            new_X["main_table"] = (table_df, table_key)
+        else:
+            table_path = [table_name]
+            is_entity = False
+
+            # Cycle twice on the relations to get all transitive relation, like:
+            # - current table name N
+            # - main table name N1
+            # - and relations: (N1, N2), (N2, N3), (N3, N)
+            # the data-path must be N2/N3/N
+            for relation in islice(cycle(X["relations"]), 2 * len(X["relations"])):
+                left, right = relation[:2]
+                if len(relation) == 3 and right == table_name:
+                    is_entity = relation[2]
+                if (
+                    left != X["main_table"]
+                    and left not in table_path
+                    and right in table_path
+                ):
+                    table_path.insert(0, left)
+            table_path = "/".join(table_path)
+            if is_entity:
+                table_data = (table_df, table_key, is_entity)
+            else:
+                table_data = (table_df, table_key)
+            new_X["additional_data_tables"][table_path] = table_data
+    return new_X
+
+
 def get_khiops_type(numpy_type):
     """Translates a numpy dtype to a Khiops dictionary type
 
@@ -417,14 +465,26 @@ def _check_input_sequence(self, X, key=None):
         # Check the key for the main_table (it is the same for the others)
         _check_table_key("main_table", key)
 
-    def _table_name_of_path(self, table_path):
-        # TODO: Add >= 128-character truncation and indexing scheme
-        return table_path.split("/")[-1]
-
     def _init_tables_from_mapping(self, X):
         """Initializes the table spec from a dict-like 'X'"""
         assert is_dict_like(X), "'X' must be dict-like"
 
+        # Detect if deprecated mapping specification syntax is used;
+        # if so, issue deprecation warning and transform it to the new syntax
+        if {"tables", "relations"}.issubset(X.keys()):
+            warnings.warn(
+                deprecation_message(
+                    "This multi-table dataset specification format",
+                    "11.0.1",
+                    replacement=(
+                        "the new data-path-based format, as documented in "
+                        ":doc:`multi_table_primer`."
+                    ),
+                    quote=False,
+                )
+            )
+            X = _upgrade_mapping_spec(X)
+
         # Check the input mapping
         check_dataset_spec(X)
 
@@ -443,7 +503,7 @@ def _init_tables_from_mapping(self, X):
             if "additional_data_tables" in X:
                 for table_path, table_spec in X["additional_data_tables"].items():
                     table_source, table_key = table_spec[:2]
-                    table_name = self._table_name_of_path(table_path)
+                    table_name = _table_name_of_path(table_path)
                     table = PandasTable(
                         table_name,
                         table_source,
@@ -460,7 +520,7 @@ def _init_tables_from_mapping(self, X):
                         parent_table_name = self.main_table.name
                     else:
                         table_path_fragments = table_path.split("/")
-                        parent_table_name = self._table_name_of_path(
+                        parent_table_name = _table_name_of_path(
                             "/".join(table_path_fragments[:-1])
                         )
                     self.relations.append(

tests/test_dataset_class.py

@@ -8,6 +8,7 @@
 import os
 import shutil
 import unittest
+import warnings
 
 import numpy as np
 import pandas as pd
@@ -16,7 +17,7 @@
 from pandas.testing import assert_frame_equal
 from sklearn import datasets
 
-from khiops.sklearn.dataset import Dataset
+from khiops.sklearn.dataset import Dataset, _upgrade_mapping_spec
 
 
 class DatasetInputOutputConsistencyTests(unittest.TestCase):
@@ -353,6 +354,99 @@ def get_ref_var_types(self, multitable, schema=None):
 
         return ref_var_types
 
+    def test_dataset_of_deprecated_mt_mapping(self):
+        """Test deprecated multi-table specification handling"""
+        (
+            ref_main_table,
+            ref_secondary_table_1,
+            ref_secondary_table_2,
+            ref_tertiary_table,
+            ref_quaternary_table,
+        ) = self.create_multitable_snowflake_dataframes()
+
+        features_ref_main_table = ref_main_table.drop("class", axis=1)
+        expected_ds_spec = {
+            "main_table": (features_ref_main_table, ["User_ID"]),
+            "additional_data_tables": {
+                "B": (ref_secondary_table_1, ["User_ID", "VAR_1"], False),
+                "B/D": (ref_tertiary_table, ["User_ID", "VAR_1", "VAR_2"], False),
+                "B/D/E": (
+                    ref_quaternary_table,
+                    ["User_ID", "VAR_1", "VAR_2", "VAR_3"],
+                ),
+                "C": (ref_secondary_table_2, ["User_ID"], True),
+            },
+        }
+        deprecated_ds_spec = {
+            "main_table": "A",
+            "tables": {
+                "A": (features_ref_main_table, "User_ID"),
+                "B": (ref_secondary_table_1, ["User_ID", "VAR_1"]),
+                "C": (ref_secondary_table_2, "User_ID"),
+                "D": (ref_tertiary_table, ["User_ID", "VAR_1", "VAR_2"]),
+                "E": (
+                    ref_quaternary_table,
+                    ["User_ID", "VAR_1", "VAR_2", "VAR_3"],
+                ),
+            },
+            "relations": {
+                ("A", "B", False),
+                ("B", "D", False),
+                ("D", "E"),
+                ("A", "C", True),
+            },
+        }
+
+        label = ref_main_table["class"]
+
+        # Test that deprecation warning is issued when creating a dataset
+        # according to the deprecated spec
+        with warnings.catch_warnings(record=True) as warning_list:
+            _ = Dataset(deprecated_ds_spec, label)
+        self.assertTrue(len(warning_list) > 0)
+        deprecation_warning_found = False
+        for warning in warning_list:
+            warning_message = warning.message
+            if (
+                issubclass(warning.category, UserWarning)
+                and len(warning_message.args) == 1
+                and "multi-table dataset specification format"
+                in warning_message.args[0]
+                and "deprecated" in warning_message.args[0]
+            ):
+                deprecation_warning_found = True
+                break
+        self.assertTrue(deprecation_warning_found)
+
+        # Test that a deprecated dataset spec is upgraded to the new format
+        ds_spec = _upgrade_mapping_spec(deprecated_ds_spec)
+        self.assertEqual(ds_spec.keys(), expected_ds_spec.keys())
+        main_table = ds_spec["main_table"]
+        expected_main_table = expected_ds_spec["main_table"]
+
+        # Test that main table keys are identical
+        self.assertEqual(main_table[1], expected_main_table[1])
+
+        # Test that main table data frame are equal
+        assert_frame_equal(main_table[0], expected_main_table[0])
+
+        # Test that additional data tables keys are identical
+        additional_data_tables = ds_spec["additional_data_tables"]
+        expected_additional_data_tables = expected_ds_spec["additional_data_tables"]
+        self.assertEqual(
+            additional_data_tables.keys(), expected_additional_data_tables.keys()
+        )
+
+        for table_path in expected_additional_data_tables:
+            table_data = additional_data_tables[table_path]
+            expected_table_data = expected_additional_data_tables[table_path]
+
+            # Test that secondary table keys are identical
+            self.assertEqual(table_data[1], expected_table_data[1])
+
+            # Test that the secondary table data frames are identical
+            assert_frame_equal(table_data[0], expected_table_data[0])
+
     def test_dataset_is_correctly_built(self):
         """Test that the dataset structure is consistent with the input spec"""
         ds_spec, label = self.create_fixture_ds_spec(