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

[ENH] Fix tests to prevent guaranteed check_estimator failure #2405

Merged
merged 4 commits into from Apr 7, 2022

Conversation

danbartl
Copy link
Collaborator

@danbartl danbartl commented Apr 7, 2022

The previous skip message led to an exception for debugging and was in alignment with @fkiraly instead replaced by None.

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

@danbartl, you pushed incomplete stuff from the reducer which breaks this PR, please remove

@danbartl
Copy link
Collaborator Author

danbartl commented Apr 7, 2022

Done.

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Thanks - a quick fix to ensure that check_estimator does not break.

@fkiraly fkiraly changed the title All forecasters fix [ENH] Fix tests to prevent guaranteed check_estimator failure Apr 7, 2022
@fkiraly fkiraly merged commit ce4df0e into sktime:main Apr 7, 2022
fkiraly added a commit that referenced this pull request Apr 9, 2022
This PR adds tests to test that tests conducted by the `check_estimator` utlity are passing, on three example estimators that are known to pass all tests in the full test suite.

This ensures that future tests catch unintentional side effects to changes in the test module that cause a `check_estimator` test to fail, such as the problem addressed recently in #2405

Also turns some instances of `pytest.skip` inside tests into `return None`, as that currently breaks `check_estimator`. An issue to replace this workaroudn has been opened here: #2419
@lmmentel lmmentel added the bugfix Fixes a known bug or removes unintended behavior label Apr 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a known bug or removes unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants