FEA allow unknowns in OrdinalEncoder transform #17406

FelixWick · 2020-06-01T15:28:07Z

Sometimes it can be convenient to allow values (categories) in the transform of OrdinalEncoder that were not present in the fit data set. For example, a machine learning method, which is able of setting such an unknown sample of the corresponding feature to a neutral value (i.e. non-informative), could use the information from all other features of the sample and still output a prediction (instead of no prediction at all).

Closes #13488
Closes #15108
Closes #16959
Closes #14534
Closes #12045
Closes #13897

FelixWick · 2020-06-01T16:14:25Z

Lots of discussion about this in here: #16959

cmarmo · 2020-06-17T16:57:11Z

Hi @scikit-learn/core-devs, this discussion was addressed during the last core-dev meeting.
I'm commenting here to draw some attention on this PR, as @FelixWick kindly proposes a simplified version that perhaps could help to move forward. Do you mind having a look before the next meeting? Thanks!

…dinalencoder

FelixWick · 2020-07-13T20:21:20Z

@thomasjpfan
Please see changes as discussed in #16959.

thomasjpfan

Thank you for the PR @FelixWick !

thomasjpfan · 2020-07-17T00:14:14Z

sklearn/preprocessing/_encoders.py

@@ -621,6 +622,18 @@ class OrdinalEncoder(_BaseEncoder):
    dtype : number type, default np.float64
        Desired dtype of output.

+    handle_unknown : 'error' or 'use_encoded_value', default='error'


Suggested change

handle_unknown : 'error' or 'use_encoded_value', default='error'

handle_unknown : {'error', 'use_encoded_value}', default='error'

thomasjpfan · 2020-07-17T00:14:36Z

sklearn/preprocessing/_encoders.py

+        When set to 'error' an error will be raised in case an unknown
+        categorical feature is present during transform. When set to
+        'use_encoded_value', the encoded value of unknown categories will be
+        set to the value given for the parameter unknown_value. In


Suggested change

set to the value given for the parameter unknown_value. In

set to the value given for the parameter `unknown_value`. In

thomasjpfan · 2020-07-17T00:16:24Z

sklearn/preprocessing/_encoders.py

+                raise TypeError("Please set unknown_value to an integer "
+                                "value.")


We can have both checks raise the same error (may need to wrap)

Suggested change

raise TypeError("Please set unknown_value to an integer "

"value.")

raise TypeError(f"Set unknown_value to an integer, got {self.unknown_value}")

thomasjpfan · 2020-07-17T00:17:51Z

sklearn/preprocessing/_encoders.py

+                if 0 <= self.unknown_value < len(self.categories_[i]):
+                    raise ValueError(f"The used value for unknown_value "
+                                     f"{self.unknown_value} is one of the "
+                                     f"values already used for encoding the "
+                                     f"seen categories.")


This validation check should be in fit after the self._fit call.

thomasjpfan · 2020-07-17T00:21:21Z

sklearn/preprocessing/_encoders.py

+            # set unknown values to None
+            if self.handle_unknown == 'use_encoded_value':
+                X_tr[:, i] = np.where(
+                    labels == self.unknown_value, None,


We can set unknown_labels = labels == self.unknown_value so the mask does not need to be computed twice.

thomasjpfan · 2020-07-17T16:03:33Z

sklearn/preprocessing/tests/test_encoders.py

@@ -553,6 +553,42 @@ def test_ordinal_encoder_raise_missing(X):
        ohe.transform(X)


+def test_ordinal_encoder_handle_unknowns():


Let's add separate test function for encoding numerical values.

thomasjpfan · 2020-07-17T16:05:57Z

sklearn/preprocessing/_encoders.py

@@ -731,6 +765,14 @@ def inverse_transform(self, X):

        for i in range(n_features):
            labels = X[:, i].astype('int64', copy=False)
-            X_tr[:, i] = self.categories_[i][labels]
+            # set unknown values to None
+            unknown_labels = labels == self.unknown_value


This can be moved into the if statement, since it is not needed with when self.handle_unknown='error'

thomasjpfan · 2020-07-17T16:08:26Z

sklearn/preprocessing/_encoders.py

+                    self.categories_[i][np.where(
+                        unknown_labels, 0, labels)])


Hmm do you think the following is clearer:

unknown_labels = labels == self.unknown_value X_tr[:, i] = self.categories_[i][np.where(unknown_labels, 0, labels)] X_tr[unknown_labels, i] = None

It might be slightly better memory wise as well.

Not sure about memory, but yes, your suggestion is clearer.

thomasjpfan · 2020-07-17T19:21:17Z

sklearn/preprocessing/tests/test_encoders.py

+def test_ordinal_encoder_handle_unknowns_numeric():
+    enc = OrdinalEncoder(handle_unknown='use_encoded_value',
+                         unknown_value=-999)
+    X_fit = np.array([[1, 7], [2, 8], [3, 9]], dtype=object)
+    X_trans = np.array([[3, 12], [23, 8], [1, 7]], dtype=object)


Suggested change

def test_ordinal_encoder_handle_unknowns_numeric():

enc = OrdinalEncoder(handle_unknown='use_encoded_value',

unknown_value=-999)

X_fit = np.array([[1, 7], [2, 8], [3, 9]], dtype=object)

X_trans = np.array([[3, 12], [23, 8], [1, 7]], dtype=object)

@pytest.mark.parametrize('dtype', [float, int])

def test_ordinal_encoder_handle_unknowns_numeric(dtype):

enc = OrdinalEncoder(handle_unknown='use_encoded_value',

unknown_value=-999)

X_fit = np.array([[1, 7], [2, 8], [3, 9]], dtype=dtype)

X_trans = np.array([[3, 12], [23, 8], [1, 7]], dtype=dtype)

(I know it is a little strange to encoding floats and ints, but these dtypes are support in OrdinalEnoder)

Nice catch. In fact, I forgot an astype in the inverse transform.

thomasjpfan · 2020-07-21T23:57:04Z

sklearn/preprocessing/_encoders.py

+                unknown_labels = labels == self.unknown_value
+                X_tr[:, i] = self.categories_[i][np.where(
+                    unknown_labels, 0, labels)]
+                X_tr = X_tr.astype(object, copy=False)


Is there a way to have only one call astype(object) similar to

scikit-learn/sklearn/preprocessing/_encoders.py

Lines 549 to 556 in ae0d535

# if ignored are found: potentially need to upcast result to

# insert None values

if found_unknown:

if X_tr.dtype != object:

X_tr = X_tr.astype(object)

for idx, mask in found_unknown.items():

X_tr[mask, idx] = None

In that cases the unknown masks are stored in a dictionary and the None are placed in X_tr after the fact.

Yeah, that might be faster.

…dinalencoder

jnothman

Otherwise LGTM!

sklearn/preprocessing/_encoders.py

jnothman · 2020-08-02T12:16:35Z

sklearn/preprocessing/tests/test_encoders.py

@@ -553,6 +553,59 @@ def test_ordinal_encoder_raise_missing(X):
        ohe.transform(X)


+def test_ordinal_encoder_handle_unknowns_string():
+    enc = OrdinalEncoder(handle_unknown='use_encoded_value', unknown_value=-2)


we should test that unknown_value=0 or 1 is also valid (but not for inverse_transform).

Not sure what you mean. Using unknown_value=0 or 1 would usually lead to an error during fit, because these are values already used for encoding the seen categories.

Why would that be an error? What's wrong with the user wanting to conflate unknowns with a specific known category? I had thought we would want to handle that case. If we won't allow that case then we should certainly test that an error is raised if it is attempted.

There is a test already for checking correct raise in case of using an unknown_value that is already used for encoding one of the categories in the fit.

And nothing wrong with conflating a specific known category with unknowns. I think it is just a matter of taste whether we want to allow conflating or not. However, I would suggest to allow it only in mode use_category (Which we said we would include in a later version to keep it small for now, see #16959.), not in use_encoded_value, because it would be clearer to explicitly state the category to be conflated. But I am fine to allow it here for use_encoded_value as well, if you prefer that (It just means deleting a few lines of code ;).).

TBH, I don't mind whether we allow or disallow it at this point, but we need to explicitly do one or the other, and test it.

Agreed. Please let me know if the test I added for this (last check in test_ordinal_encoder_handle_unknowns_raise) is not sufficient.

@thomasjpfan can you confirm whether it was your expectation to allow/disallow unknown_value=0?

I'm personally fine with being conservative and not allowing this for now. We can allow it later without breaking backward compatibility.

The test is good

jnothman · 2020-08-02T12:17:22Z

Please add an entry to the change log at doc/whats_new/v0.24.rst. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:

…dinalencoder

jnothman

Sorry, a few more things...

jnothman · 2020-08-04T10:46:39Z

sklearn/preprocessing/tests/test_encoders.py

+        enc.fit(X)
+
+    enc = OrdinalEncoder(handle_unknown='use_encoded_value', unknown_value=1)
+    msg = ("The used value for unknown_value 1 is one of the values already "


Suggested change

msg = ("The used value for unknown_value 1 is one of the values already "

msg = ("The used value for unknown_value (1) is one of the values already "

I think this is a bit less ambiguous

jnothman · 2020-08-04T10:46:56Z

sklearn/preprocessing/_encoders.py

+        When the parameter handle_unknown is set to 'use_encoded_value', this
+        parameter is required and will set the encoded value of unknown
+        categories.
+


please use .. versionadded::

jnothman · 2020-08-04T10:47:02Z

sklearn/preprocessing/_encoders.py

+        'use_encoded_value', the encoded value of unknown categories will be
+        set to the value given for the parameter `unknown_value`. In
+        :meth:`inverse_transform`, an unknown category will be denoted as None.
+


please use .. versionadded

jnothman · 2020-08-04T10:47:49Z

sklearn/preprocessing/_encoders.py

+        When the parameter handle_unknown is set to 'use_encoded_value', this
+        parameter is required and will set the encoded value of unknown
+        categories.
+
    Attributes
    ----------
    categories_ : list of arrays


This says that categories_ will correspond with the output of transform. I suspect this is no longer true. Is there a way we can make them line up?

Correct, in case of unknown categories there will be outputs of transform that are not in categories_, namely unknown_value. Are you suggesting a code change or just another description here?

I'm not sure how to change the code here. But the description, yes.

jnothman · 2020-08-04T10:48:12Z

sklearn/preprocessing/_encoders.py

+    unknown_value : int, default=None
+        When the parameter handle_unknown is set to 'use_encoded_value', this
+        parameter is required and will set the encoded value of unknown
+        categories.


please specify that unknown_value should be distinct from the value used to encode any categories.

jnothman · 2020-08-04T10:49:15Z

sklearn/preprocessing/tests/test_encoders.py

@@ -553,6 +553,59 @@ def test_ordinal_encoder_raise_missing(X):
        ohe.transform(X)


+def test_ordinal_encoder_handle_unknowns_string():
+    enc = OrdinalEncoder(handle_unknown='use_encoded_value', unknown_value=-2)


@thomasjpfan can you confirm whether it was your expectation to allow/disallow unknown_value=0?

ogrisel

LGTM (besides @jnothman's comments above). Thank you very much @FelixWick.

ogrisel · 2020-08-04T12:17:07Z

@thomasjpfan can you confirm whether it was your expectation to allow/disallow unknown_value=0?

I am not @thomasjfox :) but I wouldn't mind either option. We can always allow later if we want without breaking backward compat while the opposite would require a deprecation cycle.

I agree that the future use_category option (in the input space) is more natural to map unknown values to known categories observed on the training set.

…dinalencoder

thomasjfox · 2020-08-04T12:49:29Z

@thomasjpfan can you confirm whether it was your expectation to allow/disallow unknown_value=0?

I am not @thomasjfox :) but I wouldn't mind either option.

I'm thomasjfox, but I'm not @thomasjpfan :) I wish I wouldn't often get mentioned on scikit-learn ^^

ogrisel · 2020-08-04T12:58:10Z

Sorry @thomasjfox, github autocomplete fail...

NicolasHug

Thanks a lot @FelixWick for your consistent work!

Made some minor comments but LGTM overall.

NicolasHug · 2020-08-04T14:44:23Z

sklearn/preprocessing/_encoders.py

+        When the parameter handle_unknown is set to 'use_encoded_value', this
+        parameter is required and will set the encoded value of unknown
+        categories. It has to be distinct from the values used to encode any of
+        the categories in the fit.


Suggested change

the categories in the fit.

the categories in `fit`.

NicolasHug · 2020-08-04T14:47:41Z

sklearn/preprocessing/_encoders.py

    Attributes
    ----------
    categories_ : list of arrays
        The categories of each feature determined during fitting
        (in order of the features in X and corresponding with the output
-        of ``transform``).
+        of ``transform`` except for categories not seen during ``fit``).


I feel like adding a new sentence instead might be clearer, e.g.:

This does not include categories that weren't seen duringfit.

NicolasHug · 2020-08-04T14:48:15Z

sklearn/preprocessing/_encoders.py

@@ -678,8 +699,21 @@ def fit(self, X, y=None):
        -------
        self
        """
+        if self.handle_unknown == 'use_encoded_value':
+            if not isinstance(self.unknown_value, numbers.Integral):
+                raise TypeError(f"unknown_value should be an integer, got "


Suggested change

raise TypeError(f"unknown_value should be an integer, got "

raise TypeError(f"unknown_value should be an integer when `handle_unknown is 'use_encoded_value'`, got "

sklearn/preprocessing/_encoders.py

NicolasHug · 2020-08-04T14:50:41Z

sklearn/preprocessing/_encoders.py

+            for i in range(len(self.categories_)):
+                if 0 <= self.unknown_value < len(self.categories_[i]):


Couldn't we just use (without using range and len)

for feature_cats in self.categories_:

NicolasHug · 2020-08-04T14:58:48Z

sklearn/preprocessing/tests/test_encoders.py

@@ -553,6 +553,59 @@ def test_ordinal_encoder_raise_missing(X):
        ohe.transform(X)


+def test_ordinal_encoder_handle_unknowns_string():
+    enc = OrdinalEncoder(handle_unknown='use_encoded_value', unknown_value=-2)


I'm personally fine with being conservative and not allowing this for now. We can allow it later without breaking backward compatibility.

The test is good

NicolasHug · 2020-08-04T15:02:08Z

sklearn/preprocessing/_encoders.py

+            for i in range(len(self.categories_)):
+                X_int[~X_mask[:, i], i] = self.unknown_value


do we need to loop over features here? Can't we just do X_int[~X_mask] =self.unknown_value?

NicolasHug · 2020-08-04T15:06:28Z

sklearn/preprocessing/_encoders.py

+            if X_tr.dtype != object:
+                X_tr = X_tr.astype(object, copy=False)


Can we directly convert without checking if X_tr.dtype != object:?

NicolasHug · 2020-08-04T15:08:09Z

sklearn/preprocessing/_encoders.py

+                X_tr[:, i] = self.categories_[i][np.where(
+                    unknown_labels, 0, labels)]
+                found_unknown[i] = unknown_labels


Can we just do X_tr[:, i] = self.categories_[i][labels] as in else: case?
Since these would be overridden later anyway?

Unfortunately not, as this results in an out of bounds error.

NicolasHug · 2020-08-04T15:10:48Z

doc/whats_new/v0.24.rst

+- |Enhancement| Add value ``use_encoded_value`` for ``handle_unknown``
+  parameter and add ``unknown_value`` parameter to


Since the whole handle_unknown parameter is new, maybe this could be something like

Add a new handle_unkown parameter with a use_encoded_value option, along with a new unknown_value parameter to...

…dinalencoder

NicolasHug

Thanks @FelixWick , LGTM! The test failure seems unrelated (codecov upload fail)

NicolasHug · 2020-08-04T22:44:19Z

sklearn/preprocessing/_encoders.py

+        else:
+            if self.unknown_value is not None:


nit: we could just use elif

jnothman

Let's ship it :)

Thanks @FelixWick

agramfort · 2020-08-07T13:38:33Z

sklearn/preprocessing/_encoders.py

@@ -621,12 +622,29 @@ class OrdinalEncoder(_BaseEncoder):
    dtype : number type, default np.float64
        Desired dtype of output.

+    handle_unknown : {'error', 'use_encoded_value}', default='error'


Typo:

handle_unknown : {'error', 'use_encoded_value'}, default='error'

thx, fixed ;)

Sandy4321 · 2020-08-14T12:24:02Z

Can you clarify in which scikit version it will be available?
Meant we use. jdraines/cardinal_encoder: Implements a Scikit-Learn CardinalEncoder which differs from OrdinalEncoder in that it handles unknowns.

jnothman · 2020-08-15T11:09:23Z

This will be available in v0.24

Sandy4321 · 2020-08-17T18:36:54Z

great thanks
and in what version will be available unknowns in predictions for categorical nive Bayesian ,
meaning values not met in train data.
we wait it for long time ..

FelixWick added 2 commits June 1, 2020 16:14

allow unknowns in ordinal encoder

04b263a

change parameter name and map unknowns to -2

38456cc

github-actions bot added the module:preprocessing label Jun 1, 2020

fix merge errors

82e31d3

FelixWick mentioned this pull request Jun 1, 2020

[WIP] allow unknowns in OrdinalEncoder transform #16959

Closed

FelixWick added 3 commits July 13, 2020 20:22

Merge remote-tracking branch 'upstream/master' into allow_unknowns_or…

439b08e

…dinalencoder

included second parameter unknown_value

ce75639

indent fixes

274b999

thomasjpfan reviewed Jul 17, 2020

View reviewed changes

moved unknown value check to fit

3394490

thomasjpfan reviewed Jul 17, 2020

View reviewed changes

additional test for numeric values

6be44fa

thomasjpfan reviewed Jul 17, 2020

View reviewed changes

check for dtypes int and float

19e9474

thomasjpfan reviewed Jul 21, 2020

View reviewed changes

FelixWick added 2 commits July 30, 2020 09:22

Merge remote-tracking branch 'upstream/master' into allow_unknowns_or…

1d84ec4

…dinalencoder

only astype(object) call

72a7b75

jnothman reviewed Aug 2, 2020

View reviewed changes

FelixWick added 2 commits August 2, 2020 21:05

Merge remote-tracking branch 'upstream/master' into allow_unknowns_or…

c86a834

…dinalencoder

change log entry and cosmetics

9b5359e

FelixWick changed the title ~~[WIP] allow unknowns in OrdinalEncoder transform (simplified)~~ [MRG] allow unknowns in OrdinalEncoder transform (simplified) Aug 3, 2020

jnothman reviewed Aug 4, 2020

View reviewed changes

ogrisel approved these changes Aug 4, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into allow_unknowns_or…

d02c410

…dinalencoder

include versionadded

215ec88

more explicit description

3127688

NicolasHug reviewed Aug 4, 2020

View reviewed changes

FelixWick added 3 commits August 4, 2020 20:28

Merge remote-tracking branch 'upstream/master' into allow_unknowns_or…

41fb313

…dinalencoder

some simplifications from review

8c8b565

line too long

fd7c9fe

NicolasHug approved these changes Aug 4, 2020

View reviewed changes

cosmetics

46f8f1a

jnothman reviewed Aug 5, 2020

View reviewed changes

jnothman changed the title ~~[MRG] allow unknowns in OrdinalEncoder transform (simplified)~~ FEA allow unknowns in OrdinalEncoder transform Aug 5, 2020

jnothman merged commit 8a213df into scikit-learn:master Aug 5, 2020
7 checks passed

agramfort reviewed Aug 7, 2020

View reviewed changes

NicolasHug mentioned this pull request Aug 7, 2020

DOC minor typo fix #18119

Merged

NicolasHug mentioned this pull request Sep 16, 2020

[MRG] Support unknown_value=np.nan in OrdinalEncoder #18406

Merged

jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020

FEA allow unknowns in OrdinalEncoder transform (scikit-learn#17406)

cbea1b8

	handle_unknown : 'error' or 'use_encoded_value', default='error'
	handle_unknown : {'error', 'use_encoded_value}', default='error'

	set to the value given for the parameter unknown_value. In
	set to the value given for the parameter `unknown_value`. In

		raise TypeError("Please set unknown_value to an integer "
		"value.")

	raise TypeError("Please set unknown_value to an integer "
	"value.")
	raise TypeError(f"Set unknown_value to an integer, got {self.unknown_value}")

		@@ -553,6 +553,42 @@ def test_ordinal_encoder_raise_missing(X):
		ohe.transform(X)


		def test_ordinal_encoder_handle_unknowns():

	# if ignored are found: potentially need to upcast result to
	# insert None values
	if found_unknown:
	if X_tr.dtype != object:
	X_tr = X_tr.astype(object)

	for idx, mask in found_unknown.items():
	X_tr[mask, idx] = None

	msg = ("The used value for unknown_value 1 is one of the values already "
	msg = ("The used value for unknown_value (1) is one of the values already "

	raise TypeError(f"unknown_value should be an integer, got "
	raise TypeError(f"unknown_value should be an integer when `handle_unknown is 'use_encoded_value'`, got "

		for i in range(len(self.categories_)):
		if 0 <= self.unknown_value < len(self.categories_[i]):

		for i in range(len(self.categories_)):
		X_int[~X_mask[:, i], i] = self.unknown_value

		if X_tr.dtype != object:
		X_tr = X_tr.astype(object, copy=False)

		- \|Enhancement\| Add value ``use_encoded_value`` for ``handle_unknown``
		parameter and add ``unknown_value`` parameter to

FEA allow unknowns in OrdinalEncoder transform #17406

FEA allow unknowns in OrdinalEncoder transform #17406

Conversation

FelixWick commented Jun 1, 2020 • edited by jnothman

FelixWick commented Jun 1, 2020

cmarmo commented Jun 17, 2020

FelixWick commented Jul 13, 2020

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman commented Aug 2, 2020

jnothman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogrisel left a comment • edited

Choose a reason for hiding this comment

ogrisel commented Aug 4, 2020 • edited

thomasjfox commented Aug 4, 2020

ogrisel commented Aug 4, 2020 • edited

NicolasHug left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sandy4321 commented Aug 14, 2020

jnothman commented Aug 15, 2020 via email

Sandy4321 commented Aug 17, 2020

FelixWick commented Jun 1, 2020 •

edited by jnothman

ogrisel left a comment •

edited

ogrisel commented Aug 4, 2020 •

edited

ogrisel commented Aug 4, 2020 •

edited