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

Update tests for sklearn 1.2, server issue #1200

Merged
merged 5 commits into from
Feb 20, 2023
Merged

Update tests for sklearn 1.2, server issue #1200

merged 5 commits into from
Feb 20, 2023

Conversation

PGijsbers
Copy link
Collaborator

@PGijsbers PGijsbers commented Feb 11, 2023

A few tests got outdated with scikit-learn >= 1.2. One tests seems to fail due to the test server, for which I opened an issue here. After these fixes I get all green again (together with #1199).

@PGijsbers PGijsbers changed the base branch from update_configs to develop February 11, 2023 20:30
):
# The exact error message depends on scikit-learn version.
# Because the sklearn-extension module is to be separated,
# I will simply relax specifics of the raised Error.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be possible to test on a more relaxed regex so we can make sure that we test for the right thing?

Copy link
Collaborator

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

This looks good to me, although the tests fail because they don't clean up local parquet files. But maybe that issue can be fixed in a separate PR.

@PGijsbers PGijsbers merged commit dd62f2b into develop Feb 20, 2023
@PGijsbers PGijsbers deleted the fix_tests branch February 20, 2023 12:25
PGijsbers added a commit to Mirkazemi/openml-python that referenced this pull request Feb 23, 2023
* Relax error checking

* Skip unit test due to server issue openml/OpenML#1180

* Account for rename parameter `base_estimator` to `estimator` in sk 1.2

* Update n_init parameter for sklearn 1.2

* Test for more specific exceptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants