Encoders that are f(X, y) can produce nan results when y has non-standard index and X becomes an np.ndarray

[noahjgreen295]

Describe the bug
In encoders where the feature encoding is a function of both the feature and the target (e.g. feature_engine.encoding.MeanEncoder), resulting encoding can become all nan if:

• Feature DataFrame X is transformed upstream into an np.ndarray by a standard sklearn transformer.
  and
• Feature DataFrame X has an index other than the standard contiguous 0-based default

The error occurs in the concatenation step within the encoder, and is caused by an index mismatch resulting from the preceding array transformation.

I have tested all the feature_engine.encoding transformers and only the following three have the issue:

• MeanEncoder
• WoEEncoder
• PRatioEncoder

This was first reported by @bmreiniger here, and unit tests for fix will be based on his sample code here.

To Reproduce
Derived from @bmreiniger code here. Using feature_engine.encoding.MeanEncoder as example:

    import numpy as np
    import pandas as pd
  
    from feature_engine.encoding import MeanEncoder
    
    from sklearn.impute import SimpleImputer

    # test dataframe; setup for a transfromation where
    # coded version of 'x' will be a function of target 'y'
    df: pd.DataFrame = pd.DataFrame({
        'x': ['a', 'a', 'b', 'b', 'c', 'c'],
        'y': [1, 0, 1, 0, 1, 0],
    })
    # Key - "non-standard" index that is not the usual
    # contiguous range starting a t 0
    df.index = [101, 105, 42, 76, 88, 92]

    # Set up for standard pipeline/training etc.
    X: pd.DataFrame = df[["x"]]
    y: pd.Series = df["y"]

    # Will serve as a no-op whose chief purpose is to turn the
    # X into an np.ndarray
    si = SimpleImputer(strategy="constant", fill_value="a")

    # Encoder encodes X as a function of y; this is what
    # breaks down when X becomes an array and indexes don't accidentally match in final
    # concantenation
    encoder = MeanEncoder()

    # Sequence leading to issue:
    # 1) X becomes an array
    assert type(X) == pd.DataFrame
    X_2: np.array = si.fit_transform(X)
    assert type(X_2) == np.ndarray

    # 2) Encoder encodes as function of X, y
    df_result: pd.DataFrame = encoder.fit_transform(X_2, y)
    assert type(df_result) == pd.DataFrame

    # Assertion fails: breakdown in index matches causes results to be all nan
    assert all(df_result.iloc[:, 0].notnull())

Expected behavior
In example above, output DataFrame df_result should have meaningful encoded values, not all nan.

Desktop (please complete the following information):

• OS: Mac OS 11.6
• Versions: feature-engine 1.2.0, Python 3.9.10, Pandas 1.3.4, numpy 1.22.1

[noahjgreen295]

Am taking this issue; starting on unit test and fix now.

[noahjgreen295]

PR is here!

[solegalli]

Hi @noahjgreen295

Thanks for such a great description of the issue.

Does the OrdinalEncoder not have the issue when the encoding is set to "ordered"? it also uses y.

I see the problem and it needs fixing.

I also see that (part of) the problem is artificially introduced because of a workaround I made in the function is_dataframe to accept numpy arrays, so that I could use the battery of tests from the check_estimator from sklearn, which uses numpy arrays instead of dataframes. Feature-engine was meant to work only with dfs only. This teaches me a lesson: how introducing a workaround to use some functionality, can have an unwanted effect somewhere else :/

[solegalli]

OK scenarios:

• If X is array and y is array ==> no problem
• If X is a DataFrame and y is a series and their index match ==> no problem
• X is array (say, result of sklearn transformer) and y is a series indexed at 0 ==> no problem (am I right?)

Problematic cases:

• If X is a DataFrame and y is a series and their index does not match ==> error (but why would the index not match?)
  This is something that we should not fix, the user should be made aware of, or we should indeed raise the error.

Error scenarios:

• X is array (say, result of sklearn transformer) and y is a series not indexed at 0 ==> error (introduced by the sklearn conversion)*
• X is a dataframe not indexed at 0 and y is an array ==> error

*this error will only appear in a pipeline.

Not contemplated scenarios:

• y is multi-column array or dataframe

Hi @noahjgreen295 @bmreiniger

Did I summarize the situations correctly here? Did I miss any? or get anything wrong?

At first sight it seems that there is not a one-solution-fits-all with these scenarios. And we should probably write the checks with all of them in mind?

[noahjgreen295]

These all make sense to me, and I think they cover all possibilities.
RE your comments in the PR, I can definitely remove the portions of the logic where X could be a Series, since that is not possible.

OK scenarios:

• Everything you have looks correct to me

Problematic cases

• I can detect that and raise an error

Error scenarios

• These both seem like the scenarios we've been talking about. So for these, do you want me to automatically correct both, or just the first?

In next commits, I am removing the X as potential Series logic, and also raising an error for the problematic case. Let me know how you want me to handle the error scenarios and I'll do that next. I do feel good about merging the functionality of _check_X_y_pd_np_mismatch() into self._check_fit_input_and_variables(). However, I think it might mean some restructuring of the unit tests, so please confirm.

[noahjgreen295]

All changes done except whether to merge logic of _check_X_y_pd_np_mismatch() into another function (self._check_fit_input_and_variables()? , _is_dataframe()? etc.) or whether good as is.