rasbt / mlxtend

A library of extension and helper modules for Python's data analysis and machine learning libraries.
https://rasbt.github.io/mlxtend/
Other
4.82k stars 853 forks source link

Most recent scikit-learn results in several failed unit tests #1091

Closed it176131 closed 3 months ago

it176131 commented 3 months ago

Description

This handles the failing unit tests when scikit-learn is 1.3.1 or higher.

Related issues or pull requests

1090

1085

1087

Pull Request Checklist

it176131 commented 3 months ago

As noted in the past few commit messages, scikit-learn implemented a StackingClassifier and StackingRegressor in 0.22 with a different API. Because of this, some of the current unit tests fail as they don't expose the necessary attributes.

@rasbt, should the mlxtend versions of the StackingClassifier and StackingRegressor be replaced with the scikit-learn implementations? Or renamed (so to not cause confusion) and modified to pass the tests?

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 78.29%. Comparing base (ad06b2d) to head (d61928a). Report is 8 commits behind head on master.

Files Patch % Lines
mlxtend/classifier/stacking_cv_classification.py 50.00% 2 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1091 +/- ## ========================================== + Coverage 77.26% 78.29% +1.02% ========================================== Files 200 196 -4 Lines 11297 11140 -157 Branches 1513 1404 -109 ========================================== - Hits 8729 8722 -7 + Misses 2350 2200 -150 Partials 218 218 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rasbt commented 3 months ago

@rasbt, should the mlxtend versions of the StackingClassifier and StackingRegressor be replaced with the scikit-learn implementations? Or renamed (so to not cause confusion) and modified to pass the tests?

I would prefer to keep them as the scikit-learn port is not 1:1 equivalent, and the stacking classifiers here had some advantages (and disadvantages). I can update the unit tests to add them back, no worries!

it176131 commented 3 months ago

@rasbt, should the mlxtend versions of the StackingClassifier and StackingRegressor be replaced with the scikit-learn implementations? Or renamed (so to not cause confusion) and modified to pass the tests?

I would prefer to keep them as the scikit-learn port is not 1:1 equivalent, and the stacking classifiers here had some advantages (and disadvantages). I can update the unit tests to add them back, no worries!

Good team work! Once this is merged I'll sync my other branch and hopefully all the tests will pass 🤞