8000 [ENH] Add test for ensuring missing data handling in forecasters by HarshvirSandhu · Pull Request #7732 · sktime/sktime · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[ENH] Add test for ensuring missing data handling in forecasters #7732

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
Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

[ENH] Add test for ensuring missing data handling in forecasters #7732

wants to merge 19 commits into from

Conversation

HarshvirSandhu
Copy link
Contributor
@HarshvirSandhu HarshvirSandhu commented Feb 1, 2025 •
edited by fkiraly
Loading

Reference Issues/PRs

Fixes #7297

What does this implement/fix? Explain your changes.

Test whether forecasters with the missing data tag can handle missing data.

Does your contribution introduce a new dependency? If yes, which one?

No

What should a reviewer concentrate their feedback on?

Did you add any tests for the change?

Yes

PR checklist

For all contributions
  • I've added myself to the list of contributors with any new badges I've earned :-)
    How to: add yourself to the all-contributors file in the sktime root directory (not the CONTRIBUTORS.md). Common badges: code - fixing a bug, or adding code logic. doc - writing or improving documentation or docstrings. bug - reporting or diagnosing a bug (get this plus code if you also fixed the bug in the PR).maintenance - CI, test framework, release.
    See here for full badge reference
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.

fkiraly
fkiraly requested changes Feb 2, 2025
View reviewed changes
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.

Nice!

I would also add an else clause, and ensure the correct error message is raised, using with pytest.raises.

@fkiraly fkiraly added module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting module:tests test framework functionality - only framework, excl specific tests enhancement Adding new functionality labels Feb 2, 2025
@HarshvirSandhu
Copy link
Contributor Author

@fkiraly Anything else needed here?

fkiraly
fkiraly previously approved these changes Feb 16, 2025
View reviewed changes
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.

Looks good, great!

@HarshvirSandhu HarshvirSandhu requested a review from fkiraly May 28, 2025 06:25
@fkiraly fkiraly mentioned this pull request May 29, 2025
Open
@fkiraly fkiraly changed the title [ENH] Add test for ensuring missing data handling [ENH] Add test for ensuring missing data handling in forecasters May 29, 2025
@fkiraly fkiraly moved this to PR in progress in May - Sep 2025 mentee projects Jun 3, 2025
@phoeenniixx phoeenniixx moved this from PR in progress to PR under review in May - Sep 2025 mentee projects Jun 10, 2025
@phoeenniixx phoeenniixx moved this from PR under review to PR in progress in May - Sep 2025 mentee projects Jun 13, 2025
@HarshvirSandhu
Copy link
Contributor Author

@fkiraly One of the tests that was failing was removed in #8412

@PranavBhatP PranavBhatP moved this from PR in progress to PR under review in May - Sep 2025 mentee projects Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benHeid benHeid Awaiting requested review from benHeid benHeid is a code owner

@felipeangelimvieira felipeangelimvieira Awaiting requested review from felipeangelimvieira felipeangelimvieira is a code owner

@fnhirwa fnhirwa Awaiting requested review from fnhirwa fnhirwa is a code owner

@geetu040 geetu040 Awaiting requested review from geetu040 geetu040 is a code owner

@yarnabrina yarnabrina Awaiting requested review from yarnabrina yarnabrina is a code owner

@fkiraly fkiraly Awaiting requested review from fkiraly fkiraly is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@HarshvirSandhu HarshvirSandhu

Labels
enhancement Adding new functionality module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting module:tests test framework functionality - only framework, excl specific tests
Projects
May - Sep 2025 mentee projects 4914
Status: PR under review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ENH] TestAllForecasters should include a test for missing data handling
2 participants
@HarshvirSandhu @fkiraly
0