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.

Sign up for GitHub

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

[python-package] simplify scikit-learn 1.6+ tags support #6735

Merged
merged 4 commits into from
Dec 4, 2024
Merged

[python-package] simplify scikit-learn 1.6+ tags support #6735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0aa6c2
[python-package] simplify scikit-learn 1.6+ tags support
jameslamb Dec 3, 2024
16e1976
add more tests on __sklearn_tags__
jameslamb Dec 3, 2024
6603397
remove fallback classes
jameslamb Dec 3, 2024
f56e4ba
Merge branch 'master' into python/simplify-tags
jameslamb Dec 3, 2024
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
10 changes: 0 additions & 10 deletions python-package/lightgbm/compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@
from sklearn.utils.multiclass import check_classification_targets
from sklearn.utils.validation import assert_all_finite, check_array, check_X_y

# sklearn.utils Tags types can be imported unconditionally once
# lightgbm's minimum scikit-learn version is 1.6 or higher
try:
from sklearn.utils import ClassifierTags as _sklearn_ClassifierTags
from sklearn.utils import RegressorTags as _sklearn_RegressorTags
except ImportError:
_sklearn_ClassifierTags = None
_sklearn_RegressorTags = None
jameslamb marked this conversation as resolved.
Show resolved Hide resolved
try:
from sklearn.exceptions import NotFittedError
from sklearn.model_selection import BaseCrossValidator, GroupKFold, StratifiedKFold
Expand Down Expand Up @@ -148,8 +140,6 @@ class _LGBMRegressorBase: # type: ignore
_LGBMCheckClassificationTargets = None
_LGBMComputeSampleWeight = None
_LGBMValidateData = None
_sklearn_ClassifierTags = None
_sklearn_RegressorTags = None
_sklearn_version = None

# additional scikit-learn imports only for type hints
Expand Down
15 changes: 5 additions & 10 deletions python-package/lightgbm/sklearn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
_LGBMModelBase,
_LGBMRegressorBase,
_LGBMValidateData,
_sklearn_ClassifierTags,
_sklearn_RegressorTags,
_sklearn_version,
dt_DataTable,
pd_DataFrame,
Expand Down Expand Up @@ -726,7 +724,7 @@ def __sklearn_tags__(self) -> Optional["_sklearn_Tags"]:
# take whatever tags are provided by BaseEstimator, then modify
# them with LightGBM-specific values
return self._update_sklearn_tags_from_dict(
tags=_LGBMModelBase.__sklearn_tags__(self),
tags=super().__sklearn_tags__(),
tags_dict=self._more_tags(),
)

Expand Down Expand Up @@ -1298,10 +1296,7 @@ def _more_tags(self) -> Dict[str, Any]:
return tags

def __sklearn_tags__(self) -> "_sklearn_Tags":
tags = LGBMModel.__sklearn_tags__(self)
tags.estimator_type = "regressor"
tags.regressor_tags = _sklearn_RegressorTags(multi_label=False)
return tags
return super().__sklearn_tags__()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is necessary even though it doesn't do anything, because of this check in scikit-learn:

E TypeError: Estimator LGBMRegressor has defined either _more_tags or _get_tags, but not __sklearn_tags__. If you're customizing tags, and need to support multiple scikit-learn versions, you can implement both __sklearn_tags__ and _more_tags or _get_tags. This change was introduced in scikit-learn=1.6

https://github.com/scikit-learn/scikit-learn/blob/fba028b07ed2b4e52dd3719dad0d990837bde28c/sklearn/utils/estimator_checks.py#L4459-L4465

Added in scikit-learn/scikit-learn#30268


def fit( # type: ignore[override]
self,
Expand Down Expand Up @@ -1360,9 +1355,9 @@ def _more_tags(self) -> Dict[str, Any]:
return tags

def __sklearn_tags__(self) -> "_sklearn_Tags":
tags = LGBMModel.__sklearn_tags__(self)
tags.estimator_type = "classifier"
tags.classifier_tags = _sklearn_ClassifierTags(multi_class=True, multi_label=False)
tags = super().__sklearn_tags__()
tags.classifier_tags.multi_class = True
tags.classifier_tags.multi_label = False
return tags

def fit( # type: ignore[override]
Expand Down
6 changes: 6 additions & 0 deletions tests/python_package_test/test_sklearn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,12 @@ def test_sklearn_tags_should_correctly_reflect_lightgbm_specific_values(estimato
assert sklearn_tags.input_tags.allow_nan is True
assert sklearn_tags.input_tags.sparse is True
assert sklearn_tags.target_tags.one_d_labels is True
if estimator_class is lgb.LGBMClassifier:
assert sklearn_tags.estimator_type == "classifier"
assert sklearn_tags.classifier_tags.multi_class is True
assert sklearn_tags.classifier_tags.multi_label is False
elif estimator_class is lgb.LGBMRegressor:
assert sklearn_tags.estimator_type == "regressor"


@pytest.mark.parametrize("task", all_tasks)
Expand Down
Loading