usc-isi-i2 / dsbox-cleaning

The data cleaning TA1 component of DSBox
MIT License
6 stars 4 forks source link

Incorrect semantics of get_params/set_params #17

Closed yamsgithub closed 7 years ago

yamsgithub commented 7 years ago

Both Encoder and Imputation seem to be setting and getting hyperparameters instead of the learned parameters which is the semantics as defined by the unified interface.

Also I do not see the parameter greater_is_better being used anywhere (it is only mentioned in the doc string comments) in Imputation.

kyao commented 7 years ago

You are correct. I'll release a new version that conforms to the semantics.

Based on request from the D3M folks, we can split our imputation into multiple classes. Please, use the following classes: GreedyImputation, IterativeRegressionImputation, MICE, KNNImputation, MeanImputation

luofanghao commented 7 years ago

Thanks @kyao. @yamsgithub the parameter greater_is_better was used in the old all-in-one imputer. Now it is separated into different classes and conformed with unified interface, so no need for greater_is_better anymore.

kyao commented 7 years ago

@yamsgithub Just pushed changes with the correct get_params() and set_params(). Also, uploaded to PyPI dsbox-datacleaning-0.2.1. Please, let me know if the changes work for you.

yamsgithub commented 7 years ago

Great! thanks!