Create TargetMeanPredictor class to enhance SelectByTargetMeanPerformance by Morgan-Sell · Pull Request #358 · feature-engine/feature_engine

Morgan-Sell · 2022-01-05T01:40:17Z

SelectByTargetMeanPerformance class will call TargetMeanPredictor allowing the use of cross-validation to return the important features.

Important links:
https://scikit-learn.org/stable/glossary.html#term-predictors
https://scikit-learn.org/stable/glossary.html#term-classifier

Important: in a classifier predict() should output the real label, not 1, 0:
https://scikit-learn.org/stable/glossary.html#term-predict

check example classifier here:
https://scikit-learn.org/stable/developers/develop.html#rolling-your-own-estimator

Morgan-Sell · 2022-01-07T00:19:13Z

Need to link issues #296 and #334.

Morgan-Sell · 2022-01-10T02:40:03Z

Hi @solegalli,

I've made some progress on the TargetMeanPredictor class. In PR #334, we discussed creating the codebase so the predict() method accepts a pandas series. This causes some problems given the use of the encoders and discretisers. I've documented the problems below and suggested one potential solution.

Issues:

X for the predict() method must be a dataframe to be compatible w/ the BaseEncoder class which is the parent class to the MeanEncoder class.
If we switch input X to a dataframe w/ one column, then _check_input_matches_training_df in the BaseEncoder class returns a ValueError b/c the shape of dataframe X does not coincide with the shape of the dataframe that is used to fit the MeanEncoder().

I suspect it is cumbersome to revise all the inheritance rules/checks from the parent classes, e.g., BaseEncoder, and the encoders/discretisers that are instantiated w/in the TargetMeanPredictor class.

One solution could be to require that the user inputs a dataframe w/ the same dimensions and variable names as the dataframe that is used to fit the TargetMeanPredictor. The predict() method could have another parameter - variable_name - that instructs the predictor which pandas series to return.

Do you have any thoughts? If so, please share!

Also, the codebase is a WIP, so it's not in the best shape ;)

solegalli

Hi @Morgan-Sell

Thank you for the code start. I made a few comments. Could you take a look?

Thank you!

solegalli · 2022-01-11T14:27:16Z

Need to link issues #296 and #334.

this normally goes in the first comment you make in the PR and it takes the form:
closes #296
closes #334

conventions :p ...

solegalli

Hi @Morgan-Sell

Thank you so much for the quick turnaround with the code.

I made some suggestions on how to re-write the transform() method that should answer the issues you raised in your last comment.

If you still encounter errors, I would say, write a couple of tests, it doesn't matter that they are failing, because tests normally helps us (or at least me :p) understand where the problem is coming from.

transform(X) should take in the same dataframe that was used in fit(). Fit can take a df with multiple columns or a df with 1 column. sklearn classes do not take in pandas series, so if we have a series, we need to add .to_frame() for it to work. Please add a test using a series.to_frame() to test the functionality as well.

If something does not work or you feel stuck, don't worry, just draft some tests and we take it from there.

Thank you!

solegalli · 2022-01-11T14:48:20Z

+        the values will be sorted.
+
+    strategy: str, default='equal_width'
+        Whether to create the bins for discretization of numerical variables using


Replace

Whether the bins should be of equal width ('equal_width') or equal frequency ('equal_frequency').

Should we use an underscore or hyphen, e.g., equal_width vs equal-width? I feel like I've seen both uses or I may be confusing feature-engine and sklearn semantics. I'm adopting your pedantic tendencies ;)

trailing underscores are only added to attributes that learn parameters during the fit() method as per sklearn conventions.

I'm referring to the potential values for the self.strategy parameter. The parameter can either be equal-width or equal-distance.

Should I use (1) equal_distance and equal_width or (2) equal-distance and equal-width?

Option 1 uses an underscore. Option 2 uses a hyphen.

I suspect we want to be consistent w/ sklearn and within the feature-engine library.

solegalli · 2022-01-11T15:06:27Z

+
+
+@pytest.fixture(scope="module")
+def df_pred():


lets more this df into a conftest inside the test_prediction module.

Done. Will these dataframes only be used for predictor classes? Therefore we create a separate conftest file.

Morgan-Sell · 2022-01-12T02:43:21Z

Muchas gracias por el feedback, @solegalli! I'm still refactoring the codebase. It's a long list ;)

In the meantime, please poke and prod where you see fit!

Morgan-Sell · 2022-01-13T05:43:41Z

Hi @solegalli,

A few questions:

Given that the TargetMeanPredictor class has a mean_accuracy_score method, should the class object be able to return classification labels, e.g. binary? If so, should we build a predict_proba method?
Should we make edits to the SelectByTargetMeanPerformance class in this issue or do we create a new issue?
Other than creating a test for mean_accuracy_score, are there any other tests that we should implement?

solegalli · 2022-01-14T17:51:05Z

Hi @Morgan-Sell

Before I review, could you please try and review the files committed to this PR? I think something went wrong with rebase.

Check out main, pull upstream main to your main branch, then check out your feature branch, rebase main onto it, and then force push.

thank you!

…ariable

…sts w/in df. sucessfully printed sliced df using column names

Morgan-Sell · 2022-01-30T18:46:03Z

Hola @solegalli,

I implemented all the variable-manipulation unit tests except for test_error_find_cat_and_num_vars_datetime_var(). _find_categorical_and_numerical_variables() is returning datetime_range as a categorical variable.

I'm stuck! _find_categorical_and_numerical_variables() is working on a notebook; however, I cannot translate it to the code base.

What am I missing? :(

Also, like PR #360, the code is returning tests/test_selection/test_check_estimator_selectors.py ........Fatal Python error: Segmentation fault. Do you have any idea as to what is causing the error?

solegalli

Hi @Morgan-Sell

Well done with the code changes and the tests. This is looking really good.

I wonder if you could experiment with a few functions from sklearn that are used for classifiers and predictors and expand the tests a bit more.

See my comments below.

Thank you!

solegalli · 2022-02-02T02:07:29Z

        assert _find_all_variables(df_vartypes, non_existing_vars)
+
+
+def test_find_categorical_and_numeric_variables_one_categorical_variables(


could we put all these tests together (into 1) and use parametrize?

also, can we use a df that already exists? like df_vartypes?

It seems that the conftest dataframes are either (1) not recognized if they are not imported or (2) stay as a function object.

Is there an example of passing a conftest dataframe to @pytest.mark.parametrize?

solegalli · 2022-02-02T02:10:27Z

+        """
+        # check if 'X' is a dataframe
+        _is_dataframe(X)
+


Hi @Morgan-Sell

would you be so kind to experiment with adding this check and see if the tests still pass:

# Check that X and y have correct shape X, y = check_X_y(X, y, dtype=None)

check_X_y is from sklearn and supposedly checks that X and y fulfill some criteria.

I played w/ check_X_y. The function returns numpy arrays which lack a dtype and cause errors downstream.

I tried transforming X and y into pandas dataframe and series after check_X_y was executed. This caused ValueError: The truth value of a DataFrame is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all(). to occur for various unit tests.

I think the root of both errors exists when the MeanEncoder is applied.

solegalli · 2022-02-02T02:11:22Z

+
+        # check inf
+        _check_contains_inf(X, self.variables_numerical_)
+


if we use Check_X-y we do not need to check for na and inf ourselves. In theory. But I have not tried.

I was wondering if you could try commenting this out and see if the tests still pass?

See prior comment.

…to @pytest.mark.parameterize?

…b/c fcn returns numpy arrays which don't have dtypes

… test_attributes_upon_fitting()

Morgan-Sell · 2022-02-05T18:45:43Z

Hola @solegalli,

Shoudl we delete the tests test_target_mean_predictor_transformation() and test_regression_score_calculation_with_equal_frequency() for both the classifier and regressor? They seem redundant now that we are testing predict, predict_proba, predict_log_proba, and score for one, two and >=3 variables.

Morgan-Sell · 2022-02-09T16:51:55Z

Hola @solegalli,

I haven't heard from you in a while. I hope you're busy having fun!

Do you have any thoughts on my above questions or the classes in general? Abrazo!

solegalli · 2022-02-09T17:26:02Z

Hola @solegalli,

I haven't heard from you in a while. I hope you're busy having fun!

Do you have any thoughts on my above questions or the classes in general? Abrazo!

Yep. Sorry. Actually I was working on a different PR.

This one is a bit out of my comfort zone so I need to do some research to get more familiar with sklearn classifiers and regressors mixins. It'll take me a little longer.

solegalli reviewed Jan 10, 2022

View reviewed changes

solegalli reviewed Jan 11, 2022

View reviewed changes

sana and others added 22 commits January 14, 2022 16:28

add new folder

599f496

create TargetMeanPredictor class and its outline

b4b0dae

built more of TargetMeanPredictor class framework

18a4840

add 3 init params

359a9c0

expand fit() method

1e48118

add discretisers to fit() method

ad9f095

add 'numeric_var_startegy' param and cleaned up init()

276ef04

create new init params

122520d

identify variable types

c137b7a

instatiate encoder and discretisers in fit()

befad12

instatiate and fit encoder and discretiser

da4af56

add init params and check

95409e4

create disc_mean_dict to store means for the bins of each numerical v…

526ac84

…ariable

add checks in predict()

91dbfa6

create test_prediction directory and files

6643b0e

start creating first check

743ef44

create df_pred() in conftest.py

ceb30f3

create prediction init file and expand test_target_mean_predictor_fit()

2295a27

fix bugs

6a69106

fix bugs

ca5a754

create df_pred() to test TestMeanPredictor

e1ce27a

bug: KeyError:None when slicing df_pred even though all variables exi…

67adf9d

…sts w/in df. sucessfully printed sliced df using column names

Morgan-Sell added 4 commits January 30, 2022 10:27

fix bug in _find_categorical_and_numerical_variables()

b34f94f

try to debut test_error_find_cat_and_num_vars_datetime_var()

a9a8f3f

rename unit tests

1682d22

fix style error

4cc0250

solegalli mentioned this pull request Jan 31, 2022

Fix pytests #363

Merged

solegalli reviewed Feb 2, 2022

View reviewed changes

Morgan-Sell added 12 commits February 2, 2022 19:03

consolidating test__find_categorical_and_numeric. can a df be passed …

7424333

…to @pytest.mark.parameterize?

consolidating test__find_categorical_and_numeric. can a df be passed …

0a2cf6d

…to @pytest.mark.parameterize?

fix dataframe checks in BaseTargetMeanEstimator

8b99e66

revise dataframe checks. try sklearn check_X_y cause upstream errors …

03922c3

…b/c fcn returns numpy arrays which don't have dtypes

clean code for BaseTargetMeanEstimator and TargetMeanClassifier

5c5ee77

change df_enc_categorical_and_numeric to df_vartypes

e6ea391

delete df_enc_categorical_and_numeric

57ea390

expand test_classifier_results_with_all_categorical_variables()

cf3e05d

update test_classifier_results_with_all_numerical_variables() and fix…

1be8a71

… test_attributes_upon_fitting()

update test_regression_score_calculation_with_equal_frequency

7abb331

create test_regressor_with_two_variables()

58b6145

create 2 regressor tests

8330b0f

Morgan-Sell added 5 commits February 5, 2022 10:50

add 1 regressor test

9f610b7

fix TargetMeanClassifier fit()

d0e7040

create 2 tests for TargetMeanClassifier

2dde9e6

refactor classifier test code

74f8468

change 2 unit test names

d2364bf

solegalli mentioned this pull request Feb 18, 2022

adds get_feature_names_out() method and feature_names_in_ attribute #372

Merged

15 tasks

solegalli mentioned this pull request Mar 16, 2022

improves select by target mean functionality #390

Merged

4 tasks

solegalli closed this in #372 Mar 26, 2022

		assert _find_all_variables(df_vartypes, non_existing_vars)


		def test_find_categorical_and_numeric_variables_one_categorical_variables(


		# check inf
		_check_contains_inf(X, self.variables_numerical_)

Uh oh!

Conversation

Morgan-Sell commented Jan 5, 2022 • edited by solegalli Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Morgan-Sell commented Jan 7, 2022

Uh oh!

Morgan-Sell commented Jan 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solegalli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

solegalli commented Jan 11, 2022

Uh oh!

solegalli left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Morgan-Sell Jan 13, 2022 • 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!

Morgan-Sell commented Jan 12, 2022

Uh oh!

Morgan-Sell commented Jan 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solegalli commented Jan 14, 2022

Uh oh!

Morgan-Sell commented Jan 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solegalli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Morgan-Sell Feb 3, 2022 • 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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Morgan-Sell Feb 5, 2022 • 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!

Uh oh!

Uh oh!

Uh oh!

Morgan-Sell commented Jan 5, 2022 •

edited by solegalli

Loading

Morgan-Sell commented Jan 10, 2022 •

edited

Loading

solegalli left a comment •

edited

Loading

Morgan-Sell Jan 13, 2022 •

edited

Loading

Morgan-Sell commented Jan 13, 2022 •

edited

Loading

Morgan-Sell commented Jan 30, 2022 •

edited

Loading

Morgan-Sell Feb 3, 2022 •

edited

Loading

Morgan-Sell Feb 5, 2022 •

edited

Loading