MAINT: Update skipped tests for sklearn1.8 #2788
Conversation
david-cortes-intel
requested review from
ahuber21,
ethanglaser,
icfaust and
yuejiaointel
as code owners
|
/azp run Nightly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Looks like it fixed the issue, but note that the CI jobs still fail due to other unrelated reasons. |
|
At that point, a |
Modified to use skipif. |
tests/test_model_builders.py
Outdated
| LogisticRegression(multi_class="ovr"), | ||
| 3, | ||
| marks=pytest.mark.skipif( | ||
| not sklearn_check_version("1.8"), |
There was a problem hiding this comment.
We should do the opposite skip if sklearn_check_version("1.8")
There was a problem hiding this comment.
Yes, fixed.
Actually not possible, because it requires instantiating the model objects with the removed arguments before the test is skipped. |
| ), | ||
| ] | ||
| if not sklearn_check_version("1.8") | ||
| else [(LogisticRegression(solver="liblinear"), 2)] |
There was a problem hiding this comment.
Why do we need to remove this test, I think it's not deprecated
There was a problem hiding this comment.
The test is checking that it fails to convert the model. This would work fine.
3 participants
Description
As part of the updates for changes in sklearn1.8's logistic regression, some tests appear to have been adapted only partially. This PR removes a failing test for functionality that's being removed in sklearn1.8 and which is currently causing CI failures on the nightly jobs.
Checklist:
Completeness and readability
Testing