Skip to content

Commit

Permalink
Fix issues with sklearn 1.4 (#1045)
Browse files Browse the repository at this point in the history
* Fix issues with sklearn 1.4

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.

* Fix test after changing type of classes_

* Make CCCV test xfail for the time being

* Set xfail non-strict bc test pass with old sklearn

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.
  • Loading branch information
BenjaminBossan authored Feb 13, 2024
1 parent 0d3a8ec commit 1f7a779
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 4 deletions.
2 changes: 1 addition & 1 deletion skorch/callbacks/scoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def convert_sklearn_metric_function(scoring):

# those are scoring objects returned by make_scorer starting
# from sklearn 0.22
scorer_names = ('_PredictScorer', '_ProbaScorer', '_ThresholdScorer')
scorer_names = ('_PredictScorer', '_ProbaScorer', '_ThresholdScorer', '_Scorer')
if (
hasattr(module, 'startswith') and
module.startswith('sklearn.metrics.') and
Expand Down
4 changes: 2 additions & 2 deletions skorch/classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def classes_(self):
if not len(self.classes):
raise AttributeError("{} has no attribute 'classes_'".format(
self.__class__.__name__))
return self.classes
return np.asarray(self.classes)

try:
return self.classes_inferred_
Expand Down Expand Up @@ -301,7 +301,7 @@ def _default_callbacks(self):

@property
def classes_(self):
return [0, 1]
return np.array([0, 1])

# pylint: disable=signature-differs
def check_data(self, X, y):
Expand Down
12 changes: 11 additions & 1 deletion skorch/tests/test_classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_classes_with_gaps(self, net_cls, module_cls, data):

def test_pass_classes_explicitly_overrides(self, net_cls, module_cls, data):
net = net_cls(module_cls, max_epochs=0, classes=['foo', 'bar']).fit(*data)
assert net.classes_ == ['foo', 'bar']
assert (net.classes_ == np.array(['foo', 'bar'])).all()

def test_classes_are_set_with_tensordataset_explicit_y(
self, net_cls, module_cls, data
Expand Down Expand Up @@ -172,7 +172,12 @@ def test_pass_empty_classes_raises(
expected = "NeuralNetClassifier has no attribute 'classes_'"
assert msg == expected

@pytest.mark.xfail
def test_with_calibrated_classifier_cv(self, net_fit, data):
# TODO: This fails with sklearn 1.4.0 because CCCV does not work when
# y_proba is float32. This will be fixed in
# https://github.com/scikit-learn/scikit-learn/pull/28247, at which
# point the test should pass again and the xfail can be removed.
from sklearn.calibration import CalibratedClassifierCV
cccv = CalibratedClassifierCV(net_fit, cv=2)
cccv.fit(*data)
Expand Down Expand Up @@ -376,7 +381,12 @@ def test_default_loss_does_call_sigmoid(
net.predict_proba(X)
assert mock.call_count > 0

@pytest.mark.xfail
def test_with_calibrated_classifier_cv(self, net_fit, data):
# TODO: This fails with sklearn 1.4.0 because CCCV does not work when
# y_proba is float32. This will be fixed in
# https://github.com/scikit-learn/scikit-learn/pull/28247, at which
# point the test should pass again and the xfail can be removed.
from sklearn.calibration import CalibratedClassifierCV
cccv = CalibratedClassifierCV(net_fit, cv=2)
cccv.fit(*data)
Expand Down

0 comments on commit 1f7a779

Please sign in to comment.