-
Notifications
You must be signed in to change notification settings - Fork 915
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
fix lags_past_covariates dict type breaks lags_future_covariates when output_chunk_shift>0 #2655
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2655 +/- ##
==========================================
- Coverage 94.23% 94.18% -0.06%
==========================================
Files 141 141
Lines 15509 15510 +1
==========================================
- Hits 14615 14608 -7
- Misses 894 902 +8 ☔ View full report in Codecov by Sentry. |
…lags_future_covariates_when_output_chunk_shift
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.
Thanks for fixing this @authierj.
Would be good to add a unit test to cover this in the future :)
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.
Great job, thanks a lot @authierj. Also, congrats to your first contribution 🚀
… output_chunk_shift>0 (unit8co#2655) * if clause modified to correct bug * contribution added to changelog * implemented modifications from reviewer * unit tests added --------- Co-authored-by: “authierj” <“[email protected]”>
Checklist before merging this PR:
Fixes #2652
Summary
RegressionModel
withlags_past_covariates
as dict andlags_future_covariates
as some other type (not dict) andoutput_chunk_shift>0
, an error occurred.lags_future_covariate
is not a component lag and should therefore not be handled at any point byprocessed_component_lags
.if
statement in line 396 ofregression_model.py
fixes the bug.Other Information
None