Enhancement for the tabular validator.

[ArlindKadra]

• Refactoring the tabular validator parts.

[nabenabe0928]

Hi thanks for the PR.
I checked the implementation and suggested some changes.

[nabenabe0928]

I left just quick comments!

[nabenabe0928]

@ArlindKadra Thanks for the changes. I added some comments as suggestions to help better understanding of the codes:)

[ArlindKadra]

@ravinkohli Needs to add a fix for dropping completely empty categorical columns or the code will fail at the following line:

Auto-PyTorch/autoPyTorch/data/tabular_feature_validator.py

Line 505 in a1ed883

first_value = X[column].dropna().values[0]

[nabenabe0928]

Hi, I found some bugs, so I will send Request changes.
There are two points that I addressed:

1. We cannot judge whether a whole column can be casted to number by just looking only the first value
2. A simple change for the get_unused_category_symbol may suffer from overflow issues

[ArlindKadra]

The code for deleting null columns will fail, since on the first call to fit you will have a list of nan_columns, however, on the call to transform for train, you will not have those columns there anymore and you will make the list empty. Then, while transforming the test dataset, you will get an error because of the DataFrames with different columns.

Actually what is happening now is that we store the column order after we drop the all nan columns. So the column order is already only set after we have dropped the columns in fit. Therefore I don't think it will fail.

This is not an assumption, I noticed it failing and I debugged the trace.

[ArlindKadra]

The code for deleting null columns will fail, since on the first call to fit you will have a list of nan_columns, however, on the call to transform for train, you will not have those columns there anymore and you will make the list empty. Then, while transforming the test dataset, you will get an error because of the DataFrames with different columns.

Actually what is happening now is that we store the column order after we drop the all nan columns. So the column order is already only set after we have dropped the columns in fit. Therefore I don't think it will fail.

To be clear, you call fit with a train set, you drop the columns, you store the dropped column names.
You call transform with the train set, you have set columns but you already dropped the columns so there is no subset, you go to the else, you initialize the list of null column names to none, and you drop no columns.
You call transform with the test set, you now have an empty null column list and you have two different sized DataFrames. Is that more clear?

[ravinkohli]

The code for deleting null columns will fail, since on the first call to fit you will have a list of nan_columns, however, on the call to transform for train, you will not have those columns there anymore and you will make the list empty. Then, while transforming the test dataset, you will get an error because of the DataFrames with different columns.

Actually what is happening now is that we store the column order after we drop the all nan columns. So the column order is already only set after we have dropped the columns in fit. Therefore I don't think it will fail.

To be clear, you call fit with a train set, you drop the columns, you store the dropped column names. You call transform with the train set, you have set columns but you already dropped the columns so there is no subset, you go to the else, you initialize the list of null column names to none, and you drop no columns. You call transform with the test set, you now have an empty null column list and you have two different sized DataFrames. Is that more clear?

yes now I get it. I'll fix it. Thanks for pointing out.

[nabenabe0928]

Hi, thanks for the commits. I was a bit surprised because the files changed a lot from the last time I saw!
I put some comments and modifications, so please address them:)

[nabenabe0928]

AutoPep8 rule

Suggested change

	SUPPORTED_TARGET_TYPES = Union[
	SupportedTargetTypes = Union[

[ravinkohli]

Lets keep this a part of a separate PR later.

[nabenabe0928]

Where are we imputing now?

[ravinkohli]

we are using a sklearn imputer also for the categorical columns