You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is your feature request related to a problem? Please describe.
Currently, some feature-engine classes have more than two levels of inheritance, e.g. classes that are derived from the BaseRecursiveSelector() class.
BaseEstimator(), TransformerMixin(), and GetFeatureNamesOutMixin() --> BaseSelector() --> BaseRecursiveSelector() --> RecursiveFeatureAddition()
This many levels of abstraction significantly increase the difficulty for people to contribute.
Describe the solution you'd like
Let's limit the codebase to two levels of inheritance. There can be multiple base transformers in the _base_transformer section than each other section - e.g., discretisation, encoding, and timeseries - are limited to one base transformer.
Describe alternatives you've considered
feature-engine currently applies an alternative, i.e., more than 2 levels of inheritance, and we see the challenges that it is causing.
The text was updated successfully, but these errors were encountered:
Before we start to tackle this issue, I thought we should reconfirm we're in agreement with the approach. Of course, we're not making the "10 commandments of feature-engine", but a guideline that should be followed in the vast majority of cases.
Should we remove most of the references to the classes created in the _base_transformers directory? E.g., BaseDiscretizer should be a child of BaseEstimator and TransformerMixin, not BaseNumericalTransformer?
I suspect there will still be some references to the classes created in the _base_transformers directory; however, I expect the frequency to significantly decrease.
I believe we agreed, in general, classes like BaseImputer and BaseDiscretiser will directly inherit Scikit-learn classes like TransformerMixin and BaseEstimator.
I think we definitely need to do something about class inheritance. It is getting difficult to follow. But I'd like to read about software architecture first, and make sure that we are making a change that is going to last long.
Because we are, in essence, going to affect the entire code base. So I want to be sure about the changes that we are going to introduce.
Bottom line, I need to learn more about software design first, lol
Is your feature request related to a problem? Please describe.
Currently, some feature-engine classes have more than two levels of inheritance, e.g. classes that are derived from the BaseRecursiveSelector() class.
BaseEstimator(), TransformerMixin(), and GetFeatureNamesOutMixin() --> BaseSelector() --> BaseRecursiveSelector() --> RecursiveFeatureAddition()
This many levels of abstraction significantly increase the difficulty for people to contribute.
Describe the solution you'd like
Let's limit the codebase to two levels of inheritance. There can be multiple base transformers in the _base_transformer section than each other section - e.g., discretisation, encoding, and timeseries - are limited to one base transformer.
Describe alternatives you've considered
feature-engine currently applies an alternative, i.e., more than 2 levels of inheritance, and we see the challenges that it is causing.
The text was updated successfully, but these errors were encountered: