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

FIX Make skorch work with sklearn 1.6.0 #1076

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions skorch/classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,12 @@ def predict(self, X):
"""
return self.predict_proba(X).argmax(axis=1)

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added
tags = super().__sklearn_tags__()
tags.estimator_type = 'classifier'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should probably be inheriting from ClassifierMixin instead really.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, we don't inherit from BaseEstimator, ClassifierMixin, etc. So far, this went well but probably can cause issues like this. Would you think it is better to switch?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you're already inherriting from ClassifierMixin in this file, so adding BaseEstimator would only make things easier.

In general, we've added a substantial amount of features to the BaseEstimator, and you probably would have a much easier time if you inherit from it. These days I'd strongly recommend inheriting from the right mixins.

return tags


neural_net_binary_clf_doc_start = """NeuralNet for binary classification tasks

Expand Down Expand Up @@ -379,3 +385,9 @@ def predict(self, X):
"""
y_proba = self.predict_proba(X)
return (y_proba[:, 1] > self.threshold).astype('uint8')

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added
tags = super().__sklearn_tags__()
tags.estimator_type = 'classifier'
return tags
8 changes: 8 additions & 0 deletions skorch/net.py
Original file line number Diff line number Diff line change
Expand Up @@ -2725,3 +2725,11 @@ def __repr__(self):

parts.append(')')
return '\n'.join(parts)

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added for
# NeuralNet, NeuralNetClassifier, etc. See:
# https://scikit-learn.org/dev/developers/develop.html#estimator-tags
# https://scikit-learn.org/dev/modules/generated/sklearn.utils.Tags.html#sklearn.utils.Tags
tags = BaseEstimator.__sklearn_tags__(self)
return tags
18 changes: 18 additions & 0 deletions skorch/probabilistic.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,12 @@ def fit(self, X, y=None, **fit_params):
self.partial_fit(X, y, **fit_params)
return self

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added
tags = super().__sklearn_tags__()
tags.estimator_type = 'regressor'
return tags


gp_regr_doc_start = """Gaussian Process regressor

Expand Down Expand Up @@ -729,6 +735,12 @@ def __init__(
**kwargs
)

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added
tags = super().__sklearn_tags__()
tags.estimator_type = 'regressor'
return tags


gp_binary_clf_doc_start = """Gaussian Process binary classifier

Expand Down Expand Up @@ -912,6 +924,12 @@ def predict(self, X):
y_proba = self.predict_proba(X)
return (y_proba[:, 1] > self.threshold).astype('uint8')

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added
tags = super().__sklearn_tags__()
tags.estimator_type = 'classifier'
return tags


# BB: I could never get any reasonable results using ``SoftmaxLikelihood``. In
# fact, it always produces NaN. Probably I use it wrongly but there are no
Expand Down
6 changes: 6 additions & 0 deletions skorch/regressor.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,9 @@ def fit(self, X, y, **fit_params):
# this is actually a pylint bug:
# https://github.com/PyCQA/pylint/issues/1085
return super(NeuralNetRegressor, self).fit(X, y, **fit_params)

def __sklearn_tags__(self):
# TODO: this is just the bare minimum, more tags should be added
tags = super().__sklearn_tags__()
tags.estimator_type = 'regressor'
return tags
6 changes: 4 additions & 2 deletions skorch/tests/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ def test_grid_search_with_slds_works(
self, slds, y, classifier_module):
from sklearn.model_selection import GridSearchCV
from skorch import NeuralNetClassifier
from skorch.utils import to_numpy

net = NeuralNetClassifier(
classifier_module,
Expand All @@ -450,12 +451,13 @@ def test_grid_search_with_slds_works(
gs = GridSearchCV(
net, params, refit=False, cv=3, scoring='accuracy', error_score='raise'
)
gs.fit(slds, y) # does not raise
gs.fit(slds, to_numpy(y)) # does not raise

def test_grid_search_with_slds_and_internal_split_works(
self, slds, y, classifier_module):
from sklearn.model_selection import GridSearchCV
from skorch import NeuralNetClassifier
from skorch.utils import to_numpy

net = NeuralNetClassifier(classifier_module)
params = {
Expand All @@ -465,7 +467,7 @@ def test_grid_search_with_slds_and_internal_split_works(
gs = GridSearchCV(
net, params, refit=True, cv=3, scoring='accuracy', error_score='raise'
)
gs.fit(slds, y) # does not raise
gs.fit(slds, to_numpy(y)) # does not raise

def test_grid_search_with_slds_X_and_slds_y(
self, slds, slds_y, classifier_module):
Expand Down
Loading