Skip to content
This repository was archived by the owner on Nov 27, 2023. It is now read-only.

Fixes to unittests#11

Merged
zachary-mcpher merged 1 commit intomasterfrom
fix-failing-unittests
Aug 13, 2020
Merged

Fixes to unittests#11
zachary-mcpher merged 1 commit intomasterfrom
fix-failing-unittests

Conversation

@zachary-mcpher
Copy link
Copy Markdown

================ 663 passed, 8 skipped, 14 xfailed, 2 xpassed, 142 warnings in 214.74s (0:03:34) =================
(dask-ml-dev-skl024)

After installing Scikit-learn nightly build into dask-ml's ci latest env via:
pip install --pre --extra-index https://pypi.anaconda.org/scipy-wheels-nightly/simple scikit-learn==0.24.dev0

Copy link
Copy Markdown

@ryan-deak-zefr ryan-deak-zefr left a comment

Choose a reason for hiding this comment

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

LGTM. Are these changes backward compatible before sklearn 0.24?

Copy link
Copy Markdown

@zexuan-zhou zexuan-zhou left a comment

Choose a reason for hiding this comment

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

why do we use a dev version of sklearn instead of waiting for an official release?

estimator,
estimator_params, # parameters
scoring=make_scorer(metric, greater_is_better, needs_proba),
scoring=make_scorer(score_func=metric, greater_is_better=greater_is_better, needs_proba=needs_proba),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just checked. This should be backward compatible in sklearn 0.23.x

https://github.com/scikit-learn/scikit-learn/blob/0.23.1/sklearn/metrics/_scorer.py#L534

@zachary-mcpher
Copy link
Copy Markdown
Author

why do we use a dev version of sklearn instead of waiting for an official release?

Dask dask-ml maintainers use the dev build for their CI. See below:
https://github.com/dask/dask-ml/blob/v1.6.0/tests/model_selection/dask_searchcv/test_model_selection.py#L491

https://github.com/dask/dask-ml/blob/7f6dcb5b1082f5ef9f53217809efac21e0232f7f/dask_ml/_compat.py#L20

Dask-ml build logs here: https://dev.azure.com/dask-dev/dask/_build/results?buildId=1514&view=results

@zachary-mcpher
Copy link
Copy Markdown
Author

LGTM. Are these changes backward compatible before sklearn 0.24?

Yes they are I tested with 0.23.2 and everything passed except for the two marked xfail requiring the scikit-learn 0.24.dev0 nightly build.

@zexuan-zhou
Copy link
Copy Markdown

@zachary-mcpher zachary-mcpher merged commit eed4fe9 into master Aug 13, 2020
@zachary-mcpher zachary-mcpher deleted the fix-failing-unittests branch August 13, 2020 18:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants