-
Notifications
You must be signed in to change notification settings - Fork 275
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
657f57e
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 |
|---|---|---|
|
|
@@ -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