-
Notifications
You must be signed in to change notification settings - Fork 273
Exposing transformations applied by Cleaner and TableVectorizer
#2122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0a22b31
e93c7ec
3e1832c
36a6e71
1270c71
054ddbf
dffb2d2
4b59efd
11b6318
d2c1ef1
26abe8b
35195e2
0bb9b57
7d891c1
bcf72cc
2c698bf
4b34cd5
6d3f6c5
4bbcb87
b3580c3
2b411e0
5b3df56
6738597
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -184,6 +184,34 @@ def _get_preprocessors( | |||||||||||||||||||
| return steps | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _list_transformations(estimator, max_cols=10): | ||||||||||||||||||||
| message = "" | ||||||||||||||||||||
| post = estimator._postprocessors if hasattr(estimator, "_postprocessors") else [] | ||||||||||||||||||||
|
|
||||||||||||||||||||
| template = "{} ({} columns):\n\t- {}\n" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| for step in estimator._pipeline.named_steps: | ||||||||||||||||||||
| if step == "checkinputdataframe": | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| transformer = estimator._pipeline.named_steps[step] | ||||||||||||||||||||
| label = transformer.transformer.__class__.__name__ | ||||||||||||||||||||
| all_cols = transformer.used_inputs_ | ||||||||||||||||||||
| match transformer.transformer: | ||||||||||||||||||||
| case DropUninformative(): | ||||||||||||||||||||
| all_cols = set(transformer.all_inputs_) - set(transformer.all_outputs_) | ||||||||||||||||||||
| case CleanNullStrings(): | ||||||||||||||||||||
| label = "Null values cleaned" | ||||||||||||||||||||
| case ToFloat() if transformer in post: | ||||||||||||||||||||
| all_cols = [] | ||||||||||||||||||||
| case _: | ||||||||||||||||||||
| continue | ||||||||||||||||||||
| n_cols = len(all_cols) | ||||||||||||||||||||
| if n_cols > 0: | ||||||||||||||||||||
| columns = _limit_cols(all_cols, max_cols=max_cols) | ||||||||||||||||||||
| message += template.format(label, n_cols, "\n\t- ".join(columns)) | ||||||||||||||||||||
| return message | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class Cleaner(TransformerMixin, SkrubBaseEstimator): | ||||||||||||||||||||
| """Column-wise consistency checks and sanitization of dtypes, null values and dates. | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -541,6 +569,9 @@ def get_feature_names_out(self, input_features=None): | |||||||||||||||||||
| check_is_fitted(self, "all_outputs_") | ||||||||||||||||||||
| return np.asarray(self.all_outputs_) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def list_transformations(self, max_cols=10): | ||||||||||||||||||||
| return _list_transformations(self, max_cols=max_cols) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class TableVectorizer(TransformerMixin, SkrubBaseEstimator): | ||||||||||||||||||||
| """Transform a dataframe to a numeric (vectorized) representation. | ||||||||||||||||||||
|
|
@@ -1166,3 +1197,94 @@ def get_feature_names_out(self, input_features=None): | |||||||||||||||||||
| """ | ||||||||||||||||||||
| check_is_fitted(self, "all_outputs_") | ||||||||||||||||||||
| return np.asarray(self.all_outputs_) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def list_transformations(self, max_cols=10): | ||||||||||||||||||||
| """Returns a string reporting the transformations applied by the table | ||||||||||||||||||||
|
emassoulie marked this conversation as resolved.
|
||||||||||||||||||||
| vectorizer, and the columns they are each applied to. This covers every | ||||||||||||||||||||
| preprocessing step, each of the `numeric`, `datetime`, `low cardinality` | ||||||||||||||||||||
| and `high cardinality` transformations and any specific transformer. | ||||||||||||||||||||
|
Comment on lines
+1202
to
+1205
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. numpy formatting requires a single line
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| Parameters | ||||||||||||||||||||
| ---------- | ||||||||||||||||||||
| max_cols : int | ||||||||||||||||||||
| The maximum amount of columns to list per transformer. Any overflow is | ||||||||||||||||||||
| represented by `...` | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Returns | ||||||||||||||||||||
| ------- | ||||||||||||||||||||
| full_list : string | ||||||||||||||||||||
| An ASCII formatted message sorting transformers by category | ||||||||||||||||||||
| (preprocessing, specific processors, etc.) and listing the columns | ||||||||||||||||||||
| to which each of these transformers is applied. | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| """ | ||||||||||||||||||||
| preprocessing_transformations = ( | ||||||||||||||||||||
| "Preprocessors\n=============\n" | ||||||||||||||||||||
| + _list_transformations(self, max_cols=max_cols) | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| vectorize_transformations = "Processors by type\n==================\n" | ||||||||||||||||||||
| specific_transformations = "" | ||||||||||||||||||||
| postprocessing_transformations = "Postprocessors\n==============\n" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| all_transformers = self.kind_to_columns_.copy() | ||||||||||||||||||||
| specific = all_transformers.pop("specific") | ||||||||||||||||||||
|
|
||||||||||||||||||||
| for transformer_type, transformer_cols in all_transformers.items(): | ||||||||||||||||||||
| if transformer_cols: | ||||||||||||||||||||
|
emassoulie marked this conversation as resolved.
|
||||||||||||||||||||
| cols_to_print = list(transformer_cols) | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| For each column type (numeric, datetime etc.), there is a | ||||||||||||||||||||
| dedicated transformer in the TableVectorizer that must be | ||||||||||||||||||||
| displayed (for instance, self.numeric = Passthrough()). | ||||||||||||||||||||
| The corresponding attribute is therefore fetched | ||||||||||||||||||||
| and its class name printed. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
|
Comment on lines
+1236
to
+1242
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather have the regular comments with |
||||||||||||||||||||
| vectorize_transformations += ( | ||||||||||||||||||||
| f"{getattr(self, transformer_type).__class__.__name__} " | ||||||||||||||||||||
|
emassoulie marked this conversation as resolved.
|
||||||||||||||||||||
| f"({transformer_type} - {len(cols_to_print)} columns):" + "\n\t- " | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| vectorize_transformations += ( | ||||||||||||||||||||
| "\n\t- ".join(_limit_cols(cols_to_print, max_cols=max_cols)) + "\n" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| else: | ||||||||||||||||||||
| vectorize_transformations += ( | ||||||||||||||||||||
| f"No {transformer_type} columns have been detected." + "\n" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if self.specific_transformers: | ||||||||||||||||||||
| specific_transformations = ( | ||||||||||||||||||||
| "\nSpecific transformers\n=====================\n" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| for t in self.specific_transformers: | ||||||||||||||||||||
| specific_transformations += ( | ||||||||||||||||||||
| f"{t[0].__class__.__name__} (specific - {len(specific)} columns):" | ||||||||||||||||||||
| + "\n\t- " | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| specific_transformations += ( | ||||||||||||||||||||
| "\n\t- ".join(_limit_cols(specific, max_cols=max_cols)) + "\n" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| t_post = self._postprocessors[0] | ||||||||||||||||||||
| postprocessing_transformations += ( | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we can remove this section on postprocessing, since it's done to all columns anyway it should be mentioned clearly in the docstring (I realize now it's not) |
||||||||||||||||||||
| f"ToFloat postprocessing ({len(t_post.used_inputs_)} columns):" | ||||||||||||||||||||
| "\n\tAll float columns" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| full_list = ( | ||||||||||||||||||||
| preprocessing_transformations | ||||||||||||||||||||
| + "\n" | ||||||||||||||||||||
| + vectorize_transformations | ||||||||||||||||||||
| + specific_transformations | ||||||||||||||||||||
| + "\n" | ||||||||||||||||||||
| + postprocessing_transformations | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return full_list | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _limit_cols(col_names, max_cols=10): | ||||||||||||||||||||
| list_cols = col_names[:max_cols] | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize I suggested this version, but thinking about it again it could be simplified further to list_cols = col_names[:max_cols] + ["..."] if len(col_names) > max_cols else col_names
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. though it's not a big problem since this is wrapped into a function anyway |
||||||||||||||||||||
| if len(col_names) > max_cols: | ||||||||||||||||||||
| list_cols += ["..."] | ||||||||||||||||||||
| return list_cols | ||||||||||||||||||||
|
emassoulie marked this conversation as resolved.
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| ) | ||
| from skrub._to_float import ToFloat | ||
| from skrub._to_str import ToStr | ||
| from skrub._utils import PassThrough | ||
| from skrub.conftest import _POLARS_INSTALLED | ||
|
|
||
| MSG_PANDAS_DEPRECATED_WARNING = "Skip deprecation warning" | ||
|
|
@@ -1277,3 +1278,158 @@ def test_duration_to_float(df_module): | |
| vectorizer = Cleaner() | ||
| transformed = vectorizer.fit_transform(df) | ||
| df_module.assert_column_equal(transformed["duration"], df["duration"]) | ||
|
|
||
|
|
||
| def test_list_transformations(df_module): | ||
| def list_category(line_name, key, column_type="", with_specific=True, max_cols=3): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add a comment here to explain what the function is doing |
||
| expected_dict = { | ||
| "null": ["low_card", "datetime"] | ||
| + [f"passthrough_{i}" for i in range(1, 6)], | ||
| "uninformative": ["uninformative"], | ||
| "datetime": ["datetime"], | ||
| "float": ["numbers", "uninformative"], | ||
| "low_card": ["low_card"], | ||
| "high_card": [], | ||
| "specific": [f"passthrough_{i}" for i in range(1, 6)], | ||
| } | ||
| col_list = expected_dict[key] | ||
| if with_specific: | ||
| col_list = [x for x in col_list if x not in expected_dict["specific"]] | ||
|
|
||
| disp_list = col_list[:max_cols] | ||
|
|
||
| if len(col_list) != len(disp_list): | ||
| disp_list.append("...") | ||
|
|
||
| joiner = "" | ||
| if column_type: | ||
| joiner += " - " | ||
|
|
||
| full_list = "" | ||
| if col_list == []: | ||
| header = f"No {column_type} columns have been detected." | ||
| else: | ||
| header = f"{line_name} ({column_type}{joiner}{len(col_list)} columns):" | ||
| full_list = "\n\t- " + "\n\t- ".join(disp_list) + "\n" | ||
|
|
||
| return header + full_list | ||
|
|
||
| passthrough_line = [ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add a small comment explaining what the test dataset looks like |
||
| "red", | ||
| "orange", | ||
| "yellow", | ||
| "green", | ||
| "blue", | ||
| "indigo", | ||
| "violet", | ||
| ] | ||
|
|
||
| df_dict = { | ||
| "numbers": [1, 2, 3, 4, 5, 6, None], | ||
| "low_card": ["up", "up", "up", "down", "down", "up", "down"], | ||
| "datetime": [ | ||
| "2026-06-01", | ||
| "2026-06-04", | ||
| "2026-07-03", | ||
| "2026-05-29", | ||
| "2026-01-08", | ||
| "2026-06-20", | ||
| None, | ||
| ], | ||
| "uninformative": [False, False, False, False, False, False, False], | ||
| } | ||
| for i in range(1, 6): | ||
| df_dict[f"passthrough_{i}"] = passthrough_line | ||
|
emassoulie marked this conversation as resolved.
|
||
|
|
||
| df = df_module.make_dataframe(df_dict) | ||
|
|
||
| vectorizer = TableVectorizer( | ||
| specific_transformers=[ | ||
| (PassThrough(), [f"passthrough_{i}" for i in range(1, 6)]) | ||
| ] | ||
| ) | ||
| _ = vectorizer.fit_transform(df) | ||
| vectorizer_output = vectorizer.list_transformations(max_cols=3) | ||
|
|
||
| expected_vectorizer_output = ( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had a few issues while debugging this section because the assert at the end of the test is checking the entire string: the diff in case the expected and the true string are different is the entire thing and that makes it quite bothersome to find what the actual difference is I think this could be made simpler to parse through by splitting both strings by new line, then iterating with zip for output, expected in zip(vectorizer_output, expected_vectorizer_output):
assert output == expectedin this way the assert will trigger on the first line that differs and should print only that rather than the entire thing |
||
| "Preprocessors\n=============\n" | ||
| + list_category("Null values cleaned", "null") | ||
| + list_category("ToDatetime", "datetime") | ||
| + list_category("ToFloat", "float") | ||
| + "\nProcessors by type\n==================\n" | ||
| + list_category("PassThrough", "float", column_type="numeric") | ||
| + list_category("DatetimeEncoder", "datetime", column_type="datetime") | ||
| + list_category("OneHotEncoder", "low_card", column_type="low_cardinality") | ||
| + list_category("StringEncoder", "high_card", column_type="high_cardinality") | ||
| + "\n\nSpecific transformers\n=====================\n" | ||
| + list_category( | ||
| "PassThrough", "specific", column_type="specific", with_specific=False | ||
| ) | ||
| + "\nPostprocessors\n==============\n" | ||
| + "ToFloat postprocessing (7 columns):" | ||
| + "\n\tAll float columns" | ||
| ) | ||
| """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again this should be a regular comment, triple quotes are rendered as a string and it looks as if you were checking for equality with it rather than with |
||
| Expected output for the TableVectorizer: | ||
|
|
||
| Preprocessors | ||
| ============= | ||
| Null values cleaned (2 columns): | ||
| - low_card | ||
| - datetime | ||
| Datetime (1 columns): | ||
| - datetime | ||
| ToFloat (2 columns): | ||
| - numbers | ||
| - uninformative | ||
|
|
||
| Processors by type | ||
| ================== | ||
| PassThrough (numeric - 2 columns): | ||
| - numbers | ||
| - uninformative | ||
| DatetimeEncoder (datetime - 1 columns): | ||
| - datetime | ||
| OneHotEncoder (low_cardinality - 1 columns): | ||
| - low_card | ||
| No high_cardinality columns have been detected. | ||
|
|
||
| Specific transformers | ||
| ===================== | ||
| PassThrough (specific - 5 columns): | ||
| - passthrough_1 | ||
| - passthrough_2 | ||
| - passthrough_3 | ||
| - ... | ||
|
|
||
| Postprocessors | ||
| ============== | ||
| ToFloat postprocessing (7 columns): | ||
| All float columns""" | ||
| assert vectorizer_output == expected_vectorizer_output | ||
|
|
||
| vectorizer = Cleaner(drop_if_constant=True) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the cleaner should be moved to a separate test, or the test should be parametrized to have both the cleaner and the tablevectorizer in this case it may be simpler to have two separate tests, though that means repeating a lot of the code either way,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the reason I prefer having two separate tests (or a parametrized test) is that I was debugging this, and the test was failing on the Cleaner part I did not notice that was the case because the diff was very long, so I was looking for the failure in the part about the TableVectorizer when it was in the Cleaner |
||
| _ = vectorizer.fit_transform(df) | ||
|
|
||
| cleaner_output = vectorizer.list_transformations(max_cols=3) | ||
| expected_cleaner_output = ( | ||
| list_category("Null values cleaned", "null", with_specific=False) | ||
| + list_category("DropUninformative", "uninformative", with_specific=False) | ||
| + list_category("ToDatetime", "datetime", with_specific=False) | ||
| ) | ||
| """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment about using # rather than triple quotes |
||
| Expected output for the cleaner: | ||
| Null values cleaned (7 columns): | ||
| - low_card | ||
| - datetime | ||
| - passthrough_1 | ||
| - passthrough_2 | ||
| - passthrough_3 | ||
| - passthrough_4 | ||
| - passthrough_5 | ||
| DropUninformative (1 columns): | ||
| - uninformative | ||
| Datetime (1 columns): | ||
| - datetime | ||
| """ | ||
| assert cleaner_output == expected_cleaner_output | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment about testing line by line |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to have a docstring