Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor quantile regressor classes #61

Merged

Conversation

lbventura
Copy link
Collaborator

This is a small refactoring to avoid duplicate code at the level of CBMultiplicativeQuantileRegressor and CBAdditiveQuantileRegressor. This design (see below) could be improved though by includingCBMultiplicativeGenericRegressor and CBAdditiveGenericRegressor . This is, unfortunately, not possible ATM because the class structure relies on inheritance to implement methods. Using composition instead would likely clean up the design, I will attempt to do this in a follow-up PR.

PXL_20231110_105930560 MP

@FelixWick
Copy link
Collaborator

But you could simply add another middle base class CBGenericRegression (same hierarchy level as CBQuantileRegression) from which CBMultiplicativeGenericRegression and CBAdditiveGenericRegression would inherit (in the same way CBMultiplicativeQuantileRegression and CBAdditiveQuantileRegression are inheriting from CBQuantileRegression), couldn't you?

correct inheritance of quantile classes
@lbventura lbventura force-pushed the refactor/quantile-regressor-classes branch from 78c4067 to c4037fe Compare November 13, 2023 07:38
tests/test_integration.py Outdated Show resolved Hide resolved
cyclic_boosting/generic_loss.py Outdated Show resolved Hide resolved
@FelixWick FelixWick merged commit 43f19e5 into Blue-Yonder-OSS:main Nov 13, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants