CI Make common test xfails strict #32080
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows us to find common tests that were marked as xfail but have started passing. If a test outcome changes we either need to update our xfail list or it is a bug that needs adressing.
betatim
commented
Sep 2, 2025
| @@ -845,24 +842,6 @@ def _yield_instances_for_check(check, estimator_orig): | |||
|
|
|||
|
|
|||
| PER_ESTIMATOR_XFAIL_CHECKS = { | |||
| AdaBoostClassifier: { | |||
| # TODO: replace by a statistical test, see meta-issue #16298 | |||
There was a problem hiding this comment.
This is an instance where the check doesn't fail but I am not sure if this is because AdaBoostClassifier has been fixed or because the check is not "good enough" to detect that sample weight handling is broken?
There was a problem hiding this comment.
It's possible that the check is a bit too weak. Still we can remove the XFAIL markers for this and maybe readd it later when needed if we ever make the check stronger.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #31951
This PR enables strict xfail mode for just the common estimator checks. This allows us to find common tests that were marked as xfail but have started passing. If a test outcome changes we either need to update our xfail list or it is a bug that needs adressing.
As part of this I found some checks that no longer fail, the xfail list is updated. There are also a few estimators that are tested in different configurations, and fail/pass depending on the configuration. I've implemented that by adding custom logic to
popthe check from the xfail list for some configs.For some of the checks related to sample weights I wonder if. they really pass now or if the check is not strict enough to detect that the estimator doesn't full fill the requirements implied by the check? Maybe @ogrisel knows more about this?
ping @adrinjalali who thought it could be interesting to enable this for the scikit-learn test suite. What do you think?