create auto-detection of classifier and regressor for tree-based transformers

[Morgan-Sell]

No description provided.

[Morgan-Sell]

PR is working on issue #189 - automate detection of classifier and regressor for tree-based transformers

[Morgan-Sell]

hi @solegalli,

A couple of questions in how to approach this issue:

• Should I add an estimator variable to the DecisionTreeEncoder init() method?
• If so, then which sklearn model object should I use in the tests when instantiating DecisionTreeDiscretiser?

Gracias! Feliz ano!

[solegalli]

My idea was to use a function from sklearn that evaluates the target and decides if it is binary or continuous.

I found this function: type_of_target

The thing is, that function works well for binary or continuous variables (floats). But it is unclear for targets with just integers.

If a target has integers only, say 0,1,2,3,4, then the type_of_target will return multiclass, and we will not know if we have to train a regression or a classification, because it could be either really, depending on the real meaning of the target.

So I am not sure this issue makes sense :/

I've seen a user inadvertently train a regression model on a binary target. Maybe instead of modifying the functionality too much, we could use the type_of_target as a sanity check. If binary and is_regression-True, then raise an error, if continuous and is_regression=False, then raise an error and if multiclass, we just go with the users judgement?

Unless you can find another function that we can rely more on?

[solegalli]

If user sets is_regression=False, we could check the target with this function

If user sets is_regression=True, we use the type_of_target and if it is binary we raise an error.

Could you dig a bit in sklearn to see if there is something similar for continuous targets?

[Morgan-Sell]

Hi @solegalli,

Am I correct in that the hypothetical check_regression_target() fcn would either return discrete or continuous?

If so, one approach is that we could create an init param called discrete_threshold. Let's assume we set the default value to 5%. If variable "A" is comprised of 5% or less unique values then the function claims that the target series is discrete; otherwise, the target series is continuous.

What do you think?

For example, variable "A" has 1,000 observations and discrete_threshold is set to 5%. If variable "A" has 50 or less unique values, then variable "A" is comprised of discrete values; otherwise, the series is continuous.

I'll continue to search for a prebuilt function. However, I have not found one yet.

Feliz ano!

[solegalli]

Hi @Morgan-Sell

I looked as well, I don't think sklearn check for continuous targets.

We should not add any more parameters to the init file.

If regression is true, we just check that the target is not binary with the type_of_target. If regression is false, we use the check_classification target function from sklearn to check the target. That's all.

If target is discrete, we leave it to the user's judgement.

[Morgan-Sell]

Hi @solegalli,

I believe I implemented necessary changes - both in the init() and the test file. Lmk what you think!

[solegalli]

Hi @Morgan-Sell

Thank you so much for the code provided.

We should not rename parameters that already exist (regression, is_regression), because that creates backward incompatibility. And we avoid that as much as we can.

The target is only passed during fit, so the target checks should be moved to the fit method.

Also, try if self.regression is True instead of just if self.regression. It is more explicit. If regression was a list or a string, it would be true in the latter, but false in the former.

Thank you!

[Morgan-Sell]

@solegalli,

I implemented your suggested edits. Hopefully, we're good to go!

[solegalli]

@solegalli,

I checked the original file and you're correct about the default value and location. I changed the values.

When you say "backward compatibility" is it for functionality, consistency in appearance, or a combination of the two pending the change?

It is so that If I am using version 1.1, and I have a lot of code prepared already, and then I download version 1.2, I want to be able to run my code as is, without changing param names, order or having to now add the param because the default value has changed..

The things that you changed will affect those users who do not pass the parameter names to the class, so instead of
transformer(strategy='strategy', variables=[mylist], threshold=1], they call the transformer as transformer('strategy', [mylist],1]
or omit one of the parameters because they are using the default value, for example transformer(strategy='strategy'].

[solegalli]

I think we just need to add this to the tree discretizer, right?

Almost there :p

[Morgan-Sell]

Are you "suggesting" that we refactor the init params names/order for the DecisionTreeDiscretiser class? ;)

I never changed the init params names/order for the DecisionTreeDiscretiser class. Example code:

    def __init__(
        self,
        encoding_method: str = "arbitrary",
        cv: int = 3,
        scoring: str = "neg_mean_squared_error",
        param_grid: Optional[dict] = None,
        regression: bool = True,
        random_state: Optional[int] = None,
        variables: Union[None, int, str, List[Union[str, int]]] = None,
        ignore_format: bool = False,

    ) -> None:

        self.encoding_method = encoding_method
        self.cv = cv
        self.scoring = scoring
        self.regression = regression
        self.param_grid = param_grid
        self.random_state = random_state
        self.variables = _check_input_parameter_variables(variables)
        self.ignore_format = ignore_format

That's

[solegalli]

We need to check that the target coincides with what the user passes in the param regression also in the tree discretizer, using the functions from sklearn

[Morgan-Sell]

Will return to resolve the test failures.

[Morgan-Sell]

Hi @solegalli,

I implemented the new code and the respective tests. However, I'm receiving the following error due to the ValueError that is raised in fit():

AssertionError: The error message should contain one of the following patterns: E 0 feature\(s\) \(shape=\(\d*, 0\)\) while a minimum of \d* is required.

I traced the error back to test_all_transformers() in test_check_estimator_discretisers(), which uses sklearn's check_estimator().

I'm a bit thrown off by feature\. As far as the regex goes, I understand that \s represents any whitespace; \d is a single digit; and * is a wild card.

The error message that corresponds to the ValueError is "Trying to fit a regression to a binary target is not allowed by this transformer. Check the target values or set regression to False.". The same message is used in the DecisionTreeEncoder().

I believe the ValueError message matches the stated criteria in the AssertionError.

Do you have any ideas?

[solegalli]

Hi @Morgan-Sell

Thank you for the changes!

Regarding the first error, my suggestion would be to add regression=False to the transformer in this line and see if that resolves the issue and does not break it somewhere else. That would be the quickest (if it works)

Also, there is a style check failing:

tests/test_discretisation/test_decision_tree_discretiser.py:121:62: W292 no newline at end of file

See as well my comments below.

Thank you!

[solegalli]

why do we need this change?

[solegalli]

why do we need this change?

[solegalli]

why do we need this change?

[Morgan-Sell]

Hi @solegalli,

Of course, your suggestion worked! Why did adding regression=False to the test_check_estimator_discretisers.py file resolve the error?

I also removed the regresssion=False when instantiating the DecisionTreeEncoder() in the test file. That was a great catch! Earlier on I changed the default value of regression to True. I forgot to reset the value to regression=False. I fixed that error too.

Se acabo este PR?!

[solegalli]

Of course, your suggestion worked! Why did adding regression=False to the test_check_estimator_discretisers.py file resolve the error?

the check_estimator from sklearn creates some random X and y to test the transformers. In this case, it was creating a binary y, so because our default parameter was regression=True, and the transformer does not accept binary targets as per the changes in this PR, it was raising and error and did not allow sklearn to continue with the tests. changing regression=False, allowed the tests on the transformer to proceed with a binary target.

[solegalli]

Hi @Morgan-Sell

I made some cosmetic changes, and then broke the tests somewhere, so I had to fix and made a PR to your repo.

If you merge there, then we can merge and close here.

Thank you so much for the great work!!!

[Morgan-Sell]

Why do is df_discterise() in the test file and not the conftest file?

[solegalli]

in the general conftest we put dataframes that are used for testing across modules, by more than 1 transformer.

This df you created was exclusive to test this discretizer. So it is better to put it in the test file it is used.