feat: add accept_numeric kwarg to ToCategorical

[lisaleemcb]

The encoder ToCategorical now also accepts type int columns, in addition to str and categorical dtypes via setting the new kwarg accept_numeric to 'int'. In addition, float columns will be accepted if accept_numeric='all'.

As currently implemented, the new default is accept_numeric='int' which means that columns of strings, categoricals, and ints will be marked as categorical after performing fit_transform. Note this is different from the current default behaviour, which only accepts strings, and categoricals, and will raise an error otherwise. The current default can be recovered by setting accept_numeric=None.

The rationale for the new default is that integers are used in many datasets to represent categories. But this would make it a breaking change for direct users, so perhaps we should discuss.

However, note that this change in the default would not affect the behaviour of the TableVectorizer, as the numerical columns are currently not selected to be sent to the ToCategorical encoder. Thanks to @rcap107 for beaucoup help with understanding its behaviour.

PS Also changed the docstring to make it more concise.

[rcap107]

Thanks for the PR @lisaleemcb! There are still a few things that we need to clarify, but here is an early review with some comments

[rcap107]

This should be moved to the init of the class with the relative docstring because it's one of the requirements of scikit-learn transformers

diff --git a/skrub/_to_categorical.py b/skrub/_to_categorical.py
index de42d0683..1f17803fc 100644
--- a/skrub/_to_categorical.py
+++ b/skrub/_to_categorical.py
@@ -158,7 +158,10 @@ class ToCategorical(SingleColumnTransformer):
     True
     """
 
-    def fit_transform(self, column, accept_numeric="int", y=None):
+    def __init__(self, accept_numeric="int"):
+        self.accept_numeric = accept_numeric
+
+    def fit_transform(self, column, y=None):
         """Fit the encoder and transform a column.
 
         Parameters
@@ -166,12 +169,6 @@ class ToCategorical(SingleColumnTransformer):
         column : pandas or polars Series
             The input to transform.
 
-        accept_numeric : str, default="int"
-            How to handle numeric columns. If "int", will convert integer
-            columns to categorical. If "all", both float and integer columns
-            will be accepted. If None, no numeric
-            columns will be accepted.
-
         y : None
             Ignored.

[rcap107]

This set of if-else conditions can be simplified (a bit) by combining the conditions that return sbd.to_categorical. It's not a big simplification, but it does save some duplication.

@@ -182,19 +179,19 @@ class ToCategorical(SingleColumnTransformer):
         """
         self.all_outputs_ = [sbd.name(column)]
 
+
         if sbd.is_categorical(column):
             return column
-        elif sbd.is_string(column):
-            return sbd.to_categorical(column)
-        elif sbd.is_integer(column) and accept_numeric in ("int", "all"):
-            return sbd.to_categorical(column)
-        elif sbd.is_float(column) and accept_numeric == "all":
+        if (
+            sbd.is_string(column)
+            or (sbd.is_integer(column) and self.accept_numeric in ("int", "all"))
+            or (sbd.is_float(column) and self.accept_numeric == "all")
+        ):
             return sbd.to_categorical(column)
-        else:
-            raise RejectColumn(
-                f"Column {sbd.name(column)!r} does not contain strings "
-                "or numerical data (if accept_numeric is 'int' or 'all')."
-            )
+        raise RejectColumn(
+            f"Column {sbd.name(column)!r} does not contain strings "
+            "or numerical data (if accept_numeric is 'int' or 'all')."
+        )
 
     def transform(self, column):
         """Transform a column.

[rcap107]

here the asserts should check for equality with an expected column, which you can get by doing

expected = sbd.to_categorical(i)
df_module.assert_column_equal(transformed, expected)

that way we know both that the code ran correctly, and that the output is what we're expecting to see

[rcap107]

expected failures should be wrapped in their own test, and the test should be parametrized for float and int cases