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

[python] Migrate to parametrize_with_checks for scikit-learn integration tests #2947

Closed
rth opened this issue Mar 26, 2020 · 2 comments
Closed

Comments

@rth
Copy link

rth commented Mar 26, 2020

Minor maintenance point but it could be worth migrating to parametrize_with_checks instead of using check_estimator directly in scikit-learn integration tests (or rather the private _yield_all_checks function from scikit-learn):

def test_sklearn_integration(self):
# we cannot use `check_estimator` directly since there is no skip test mechanism
for name, estimator in ((lgb.sklearn.LGBMClassifier.__name__, lgb.sklearn.LGBMClassifier),
(lgb.sklearn.LGBMRegressor.__name__, lgb.sklearn.LGBMRegressor)):
check_parameters_default_constructible(name, estimator)
# we cannot leave default params (see https://github.com/microsoft/LightGBM/issues/833)
estimator = estimator(min_child_samples=1, min_data_in_bin=1)
for check in _yield_all_checks(name, estimator):

It particular, this would make each pair of estimator-check an independent pytest check which will make them easier to skip or debug issues like #2628

Reference : https://scikit-learn.org/stable/developers/develop.html#rolling-your-own-estimator

@rth rth changed the title [python] Migrate to parametrize_with_checks for scikit-learn's check_estimators [python] Migrate to parametrize_with_checks for scikit-learn integration tests Mar 26, 2020
@StrikerRUS
Copy link
Collaborator

Thanks for letting us know about parametrize_with_checks! I believe that with estimator tags mechanism it'll simplify some code.

I'm adding this to our requests hub.

@StrikerRUS
Copy link
Collaborator

Closed in favor of being in #2302. We decided to keep all feature requests in one place.

Welcome to contribute this feature! Please re-open this issue (or post a comment if you are not a topic starter) if you are actively working on implementing this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants