Skip to content

_xfail_tests -> _xfail_checks & revert changes to check_estimator #17175

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

Closed
wants to merge 3 commits into from

Conversation

rth
Copy link
Member

@rth rth commented May 10, 2020

Fixes CI and some outdated parts from #16507 merged by accident.

Reverts check_estimator to its previous version on master, since the xfail estimators tags were better addressed in earlier PRs.

# skip tests marked as a known failure and raise a warning
msg = xfail_checks[check_name]
warnings.warn(f'Skipping {check_name}: {msg}', SkipTestWarning)
continue
Copy link
Member Author

@rth rth May 10, 2020

Choose a reason for hiding this comment

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

All changes to check_estimator revert the modifications in #16507 since they are no longer relevant.

@@ -478,7 +478,7 @@ def _compute_score_samples(self, X, subsample_features):

def _more_tags(self):
return {
'_xfail_test': {
'_xfail_checks': {
Copy link
Member Author

Choose a reason for hiding this comment

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

The name of the tag was changed on master

@rth
Copy link
Member Author

rth commented May 10, 2020

In the end there does look like some changes to check_estimator for xfail are necessary. Reverted #16507 on master to fix CI and will make a new PR with the updated changes.

@rth rth closed this May 10, 2020
@NicolasHug
Copy link
Member

ping me on the new one please ;)

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