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

Jump to bottom

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
80f1ae1
Relax error checking
PGijsbers Feb 11, 2023
74f11d2
Skip unit test due to server issue openml/openml#1180
PGijsbers Feb 11, 2023
016e7ba
Account for rename parameter `base_estimator` to `estimator` in sk 1.2
PGijsbers Feb 11, 2023
8dcb9ee
Update n_init parameter for sklearn 1.2
PGijsbers Feb 11, 2023
2388525
Test for more specific exceptions
PGijsbers Feb 20, 2023
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
46 changes: 22 additions & 24 deletions tests/test_extensions/test_sklearn_extension/test_sklearn_extension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,14 +338,15 @@ def test_serialize_model_clustering(self):
)
)
else:
n_init = '"warn"' if LooseVersion(sklearn.__version__) >= "1.2" else "10"
fixture_parameters = OrderedDict(
(
("algorithm", '"lloyd"'),
("copy_x", "true"),
("init", '"k-means++"'),
("max_iter", "300"),
("n_clusters", "8"),
("n_init", "10"),
("n_init", n_init),
("random_state", "null"),
("tol", "0.0001"),
("verbose", "0"),
Expand All @@ -358,13 +359,13 @@ def test_serialize_model_clustering(self):
)
structure = serialization.get_structure("name")

self.assertEqual(serialization.name, fixture_name)
self.assertEqual(serialization.class_name, fixture_name)
self.assertEqual(serialization.custom_name, fixture_short_name)
self.assertEqual(serialization.description, fixture_description)
self.assertEqual(serialization.parameters, fixture_parameters)
self.assertEqual(serialization.dependencies, version_fixture)
self.assertDictEqual(structure, fixture_structure)
assert serialization.name == fixture_name
assert serialization.class_name == fixture_name
assert serialization.custom_name == fixture_short_name
assert serialization.description == fixture_description
assert serialization.parameters == fixture_parameters
assert serialization.dependencies == version_fixture
assert structure == fixture_structure

def test_serialize_model_with_subcomponent(self):
model = sklearn.ensemble.AdaBoostClassifier(
Expand Down Expand Up @@ -1449,22 +1450,19 @@ def test_deserialize_complex_with_defaults(self):
pipe_orig = sklearn.pipeline.Pipeline(steps=steps)

pipe_adjusted = sklearn.clone(pipe_orig)
if LooseVersion(sklearn.__version__) < "0.23":
params = {
"Imputer__strategy": "median",
"OneHotEncoder__sparse": False,
"Estimator__n_estimators": 10,
"Estimator__base_estimator__n_estimators": 10,
"Estimator__base_estimator__base_estimator__learning_rate": 0.1,
}
else:
params = {
"Imputer__strategy": "mean",
"OneHotEncoder__sparse": True,
"Estimator__n_estimators": 50,
"Estimator__base_estimator__n_estimators": 10,
"Estimator__base_estimator__base_estimator__learning_rate": 0.1,
}
impute_strategy = "median" if LooseVersion(sklearn.__version__) < "0.23" else "mean"
sparse = LooseVersion(sklearn.__version__) >= "0.23"
estimator_name = (
"base_estimator" if LooseVersion(sklearn.__version__) < "1.2" else "estimator"
)
params = {
"Imputer__strategy": impute_strategy,
"OneHotEncoder__sparse": sparse,
"Estimator__n_estimators": 10,
PGijsbers marked this conversation as resolved.
Show resolved Hide resolved
f"Estimator__{estimator_name}__n_estimators": 10,
f"Estimator__{estimator_name}__{estimator_name}__learning_rate": 0.1,
}

pipe_adjusted.set_params(**params)
flow = self.extension.model_to_flow(pipe_adjusted)
pipe_deserialized = self.extension.flow_to_model(flow, initialize_with_defaults=True)
Expand Down
12 changes: 8 additions & 4 deletions tests/test_runs/test_run_functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,13 @@ def test_check_erronous_sklearn_flow_fails(self):

# Invalid parameter values
clf = LogisticRegression(C="abc", solver="lbfgs")
with self.assertRaisesRegex(
ValueError,
r"Penalty term must be positive; got \(C=u?'abc'\)", # u? for 2.7/3.4-6 compability
):
# 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?

# old: r"Penalty term must be positive; got \(C=u?'abc'\)"
# new: sklearn.utils._param_validation.InvalidParameterError:
# The 'C' parameter of LogisticRegression must be a float in the range (0, inf]. Got 'abc' instead. # noqa: E501
with self.assertRaises(Exception):
openml.runs.run_model_on_task(
task=task,
model=clf,
Expand Down Expand Up @@ -680,6 +683,7 @@ def get_ct_cf(nominal_indices, numeric_indices):
sentinel=sentinel,
)

@unittest.skip("https://github.com/openml/OpenML/issues/1180")
@unittest.skipIf(
LooseVersion(sklearn.__version__) < "0.20",
reason="columntransformer introduction in 0.20.0",
Expand Down