-
Notifications
You must be signed in to change notification settings - Fork 87
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
Update split_data to call split_multiseries_data #4312
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4312 +/- ##
=======================================
+ Coverage 99.7% 99.7% +0.1%
=======================================
Files 357 357
Lines 39739 39767 +28
=======================================
+ Hits 39619 39647 +28
Misses 120 120
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some quick comments but otherwise LGTM
@@ -29,6 +30,8 @@ def test_split_data( | |||
X, y = X_y_regression | |||
problem_configuration = None | |||
if is_time_series(problem_type): | |||
if is_multiseries(problem_type): | |||
pytest.skip("Multiseries time series is tested separately") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of skipping could we mock out the multi series split and asset that it's called?
Closes #4311