aimclub / FEDOT

Automated modeling and machine learning framework FEDOT
https://fedot.readthedocs.io
BSD 3-Clause "New" or "Revised" License
627 stars 86 forks source link

Options to improve design in fedot.api module #422

Closed J3FALL closed 2 years ago

J3FALL commented 2 years ago

After the last merge related with API refactoring i found some issues with design.

I think suffix *Helper does not bring much details for the user or developer. In fact, most of the classes help users to do something. In some cases the suffix could be ommited without the loss of meaning: ApiInitialAssumptionsHelper -> InitialAssumptions. Ofc it's a holy war about naming, but some good points about 'Why Manager/Helper/etc are bad?" can be read here.

It is also related to the previous example: ApiFacade is derived of several interfaces (ApiDataHelper, ApiComposerHelper, ApiMetricsHelper, ApiInitialAssumptionsHelper). However, ApiComposerHelper is also derived from (ApiMetricsHelper, ApiInitialAssumptionsHelper). The abstract scheme of classes is like: image

So it looks like a diamond problem and this could be a mess in a future. I suggest to get rid of multiple inheritance here and implement it via composition.

Examples:

image image

Maybe it would be useful to add some kind-of pre-commit hooks in order to check PEP issues? Also we could think about our pycharm configuration for formatting and add it to the contribution guide.

nicl-nno commented 2 years ago

image

Аlso, obsolete 'chain' naming is still exists.

Dreamlone commented 2 years ago

Can we close this issue after pull request #484?

J3FALL commented 2 years ago

Can we close this issue after pull request #484?

Looks like, yes.