-
Notifications
You must be signed in to change notification settings - Fork 394
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
Fix issues with sklearn 1.4 #1045
Conversation
DON'T MERGE YET. Some skorch tests fail with sklearn v1.4. This commit fixes them: 1. Inheritance structure of scorers seems to have changed. 2. classes_ attribute should always be numpy array 3. CalibratedClassifierCV only works with predict_proba being float64 To fix the latter, I'm now casting the output of predict_proba to float64. However, I'm not sure if this is a good idea, as it might break existing code. I'm just adding it in for now to check if the tests pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just keep float32 and skip the tests until the sklearn fix is released
I am +1 on waiting for the release.
For Python 3.8, an older sklearn version (1.3.2) is installed, which does not result in the test failing. Therefore, instead of requiring a strict xfail, make xfail optional.
Hey @BenjaminBossan, I've just run into issues 2 and 3 from here. Is there likely to be a new skorch release including these soon, or would I be best to install from master to get the fixes for now? |
@foster999 Good point, thanks. I also just tested scikit-learn 1.5.0 and this time around, there don't seem to be any regressions. Release will come very soon. |
Excellent, thanks for the quick response! Shall keep my eyes peeled for a release 🚀 |
@foster999 Release is out (v1.0.0 no less) |
Thanks for the heads up @BenjaminBossan ❤️ |
Some skorch tests fail with sklearn v1.4. This commit fixes them:
classes_
attribute should always be numpy arrayCalibratedClassifierCV
only works with the output ofpredict_proba
being float64To fix the latter, I'm now casting the output of
predict_proba
to float64. However, I'm not sure if this is a good idea, as it might break existing code. I'm just adding it in for now to check if the tests pass.Update: So the tests pass. Regarding the issue with
CalibratedClassifierCV
and float32, here are some suggestions:NeuralNet
to optionally casty_proba
to float64.Personally, I'm in favor of 2.