Use handle_unknown=ignore in SuperVectorizer by LeoGrin · Pull Request #473 · skrub-data/skrub

LeoGrin · 2023-01-23T14:55:06Z

Change default low_card_cat_transformer in SuperVectorizer to use handle_unknown="ignore".

Solves #455

LeoGrin · 2023-01-23T15:09:04Z

Hmm it seems that we can't both specify drop='if_binary, and handle_unknown="ignore", because it would create some ambiguity on what a zero vector means. I would say it's better to keep what we have now, that is drop='if_binary and handle_unknown="error", but I'd like to hear other opinions.

GaelVaroquaux · 2023-01-23T15:21:13Z

Hmm it seems that we can't both specify drop='if_binary, and handle_unknown= "ignore", because it would create some ambiguity on what a zero vector means. I would say it's better to keep what we have now, that is drop='if_binary and handle_unknown="error", but I'd like to hear other opinions.

I would rather have a pipeline that runs and does not raise error, from a practical point of view.

jovan-stojanovic

Thanks, just a typo to correct I think.

GaelVaroquaux · 2023-01-30T07:56:20Z

  :pr:`432` by :user:`Lilian Boulard <LilianBoulard>`

+* The :class:`SuperVectorizer`'s default `OneHotEncoder` for low cardinality categorical variables now defaults to `handle_unknown="ignore"` and `drop=None` instead of `handle_unknown="error"` and `drop=if_binary`. This means
+  that categories seen only at test time will be encoded by a vector of zeroes instead of raising an error, and that no category will be dropped. :pr:`473` by :user:`Leo Grinsztajn <LeoGrin>`


Why did we get rid of "drop=if_binary"? It had been added as a specific request to avoid having too big encoded matrices.

I understand that the semantics of the two options somewhat clash (maybe something to discuss with the scikit-learn team) and bad things will happen if we have binary and unknown in the same column, but it is quite unlikely.

Potential solutions to keep using drop="binary":

keep handle_unknown="ignore" for non-binary columns, but use handle_unknown="error" for binary columns (@GaelVaroquaux prefered solution I think)

create a subclass of OneHotEncoder which maps new columns encountered during transform to "infrequent" (quite similar to infrequent_if_exists but without needing sklearn > 1.1).

Tagging some scikit-learn people to have other opinions: @glemaitre @Vincent-Maladiere

I think this is related to: scikit-learn/scikit-learn#18072 and resolved in scikit-learn/scikit-learn#19041.

I think that the argument taken was the following: scikit-learn/scikit-learn#18072 (comment)

In the end, we are lenient and we are not capable of inverting properly the array.

I think that being lenient is fine for our purposes (the case is going to be very infrequent) and thus we should just turn on both options.

Thanks @glemaitre !

Thanks @glemaitre ! I also think that that's the best solution. The only issue is that sklearn is lenient from 0.24.2, so we get an error for earlier version (from 0.23.0). I think we can just catch it.

What I finally chose is to just turn on both options, but to use handle_ignore="error" if sklearn version is < 0.24.2, while issuing a warning. I know that we excluded using infrequent_if_exists because changing behaviour for different sklearn version might be confusing for users, but I figured it would be okay in this case, as it only concerns sklearn versions between 0.23 and 0.24.2.

GaelVaroquaux · 2023-01-30T21:36:52Z

Actually I've deleted this and converted every categorical column to object dtype,

Often, in pandas things are faster (and consume less memory) with categorical dtype, rather than object dtype. It is in general a good idea to stick to categorical dtype.

GaelVaroquaux · 2023-01-30T21:42:29Z

What I finally chose is to just turn on both options, but to use handle_ignore= "error" if sklearn version is < 0.24.2, while issuing a warning.

Might lead to hacky code to issue the warning only for the right case.

Change default `low_card_cat_transformer` in SuperVectorizer to use handle_unknown="ignore"

Pandas `category` dtype conversion converts new categories to nans, so we now update the list of categories before converting.

Co-authored-by: Jovan Stojanovic <62058944+jovan-stojanovic@users.noreply.github.com>

This avoids dealing with the categories attached to the dtype.

And use handle_unknown="error" for sklearn < 0.24.2.

…ectorizer" This reverts commit 34ed05f.

LeoGrin · 2023-02-13T10:44:45Z

Often, in pandas things are faster (and consume less memory) with categorical dtype, rather than object dtype. It is in general a good idea to stick to categorical dtype.

Ah ok, I've reverted the change to keep categorical columns. If the speed gain is significant, would it make sense to convert columns with string or object dtypes to categorical? (maybe if the number of category is not too high)

LeoGrin · 2023-02-13T10:51:29Z

What I finally chose is to just turn on both options, but to use handle_ignore= "error" if sklearn version is < 0.24.2, while issuing a warning.
Might lead to hacky code to issue the warning only for the right case.

Right now I issue a warning in _clone_transformers (called at the beginning of fit) if low_card_cat_encoder is None (default). So there may be a warning a bit more often what we could have (if we catched the OneHotEncoder errors for earlier sklearn versions), but on the other hand we avoid hacky code.

and change the warning message to be more informative.

GaelVaroquaux

A couple of small changes are necessary, but the overall strategy is the right one. Thanks!!!

GaelVaroquaux

I'd like a bit more name change :(

GaelVaroquaux

Excellent! Thank you!!

GaelVaroquaux · 2023-02-16T06:23:46Z

Merging!

jovan-stojanovic reviewed Jan 25, 2023

View reviewed changes

Comment thread dirty_cat/_super_vectorizer.py Outdated

jovan-stojanovic approved these changes Jan 28, 2023

View reviewed changes

GaelVaroquaux reviewed Jan 30, 2023

View reviewed changes

Comment thread dirty_cat/_super_vectorizer.py

jovan-stojanovic added this to the 0.4.0 release milestone Feb 1, 2023

LeoGrin and others added 10 commits February 13, 2023 11:18

Use handle_unknown=ignore in SuperVectorizer

5577292

Change default `low_card_cat_transformer` in SuperVectorizer to use handle_unknown="ignore"

Update changelog

fe547ad

Change drop to None

482ae31

Fix bug for new categories for categorical columns

e08ce7a

Pandas `category` dtype conversion converts new categories to nans, so we now update the list of categories before converting.

Fix test to prevent n_samples < n_components

87145c1

Update dirty_cat/_super_vectorizer.py

1bc9581

Co-authored-by: Jovan Stojanovic <62058944+jovan-stojanovic@users.noreply.github.com>

Convert all categorical columns to object dtype inside SuperVectorizer

34ed05f

This avoids dealing with the categories attached to the dtype.

Put back drop="if_binary"

d9f9de9

And use handle_unknown="error" for sklearn < 0.24.2.

Revert "Convert all categorical columns to object dtype inside SuperV…

7fc3950

…ectorizer" This reverts commit 34ed05f.

finish merge

ab27660

LeoGrin force-pushed the handle_unknown_sv branch from e726d92 to ab27660 Compare February 13, 2023 10:42

change name in CHANGES.rst

5276c19

LeoGrin mentioned this pull request Feb 13, 2023

Use categorical dtypes for low cardinality variables? #488

Closed

1 task

Change min version for handle_unknown=ignore to 1.0.0

09faad3

and change the warning message to be more informative.

LeoGrin requested a review from jovan-stojanovic February 13, 2023 17:57

GaelVaroquaux reviewed Feb 14, 2023

View reviewed changes

Comment thread dirty_cat/_table_vectorizer.py Outdated

GaelVaroquaux reviewed Feb 14, 2023

View reviewed changes

Comment thread dirty_cat/tests/test_table_vectorizer.py Outdated

GaelVaroquaux requested changes Feb 14, 2023

View reviewed changes

warning stacklevel + fix name

b3550fe

LeoGrin requested review from GaelVaroquaux and removed request for jovan-stojanovic February 15, 2023 10:18

GaelVaroquaux requested changes Feb 15, 2023

View reviewed changes

Comment thread dirty_cat/tests/test_table_vectorizer.py Outdated

replace sup_vec by table_vec

da8446b

GaelVaroquaux approved these changes Feb 16, 2023

View reviewed changes

GaelVaroquaux merged commit 9b15dd2 into skrub-data:main Feb 16, 2023

Conversation

LeoGrin commented Jan 23, 2023

Uh oh!

LeoGrin commented Jan 23, 2023

Uh oh!

GaelVaroquaux commented Jan 23, 2023 via email

Uh oh!

jovan-stojanovic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GaelVaroquaux Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

LeoGrin Jan 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glemaitre Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

LeoGrin Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

LeoGrin Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GaelVaroquaux commented Jan 30, 2023 via email

Uh oh!

GaelVaroquaux commented Jan 30, 2023 via email

Uh oh!

LeoGrin commented Feb 13, 2023

Uh oh!

LeoGrin commented Feb 13, 2023

Uh oh!

Uh oh!

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux commented Feb 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LeoGrin Jan 30, 2023 •

edited

Loading