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

Change MRO #879

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Change MRO #879

merged 1 commit into from
Nov 8, 2024

Conversation

jtilly
Copy link
Member

@jtilly jtilly commented Nov 8, 2024

Closes #878.

In CI against the latest scikit-learn version, we're seeing

FAILED tests/glm/test_glm.py::test_check_estimator[GeneralizedLinearRegressor-kwargs0] - AssertionError: GeneralizedLinearRegressor is inheriting from mixins in the wrong order. In general, in mixin inheritance, more specialized mixins must come before more general ones. This means, for instance, `BaseEstimator` should be on the right side of most other mixins. You need to change the order so that:
RegressorMixin comes before/left side of BaseEstimator
FAILED tests/glm/test_glm.py::test_check_estimator[GeneralizedLinearRegressorCV-kwargs1] - AssertionError: GeneralizedLinearRegressorCV is inheriting from mixins in the wrong order. In general, in mixin inheritance, more specialized mixins must come before more general ones. This means, for instance, `BaseEstimator` should be on the right side of most other mixins. You need to change the order so that:
RegressorMixin comes before/left side of BaseEstimator

This check was introduced with scikit-learn/scikit-learn#30234.

Was there are reason we ordered things they way we did or is this safe to change?

@lbittarello lbittarello merged commit fb863f4 into main Nov 8, 2024
22 checks passed
@lbittarello lbittarello deleted the issue-878 branch November 8, 2024 08:52
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.

Daily run failure: Unit tests
2 participants