-
Notifications
You must be signed in to change notification settings - Fork 881
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
return single encoded covariates if called on single target #1193
Conversation
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.
good for me
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 @dennisbader.
Do you know what was the impact? Were the encoders simply not working for single series (though I guess we would have cought it earlier)?
Reason was that SequentialEncoder always returned a list of TimeSeries even for single series input (@dumjax found this when testing for Shap integration). For TorchForecastingModels this wasn't an issue as the series were internally converted to a list of series before using the encoders. I only added RegressionModel encoder tests for the multi-ts case, which is why they didn't catch it. |
If you have a moment, this is always welcome, so we make sure the issue does not reproduce again. |
As discussed, still to do:
|
I will open a new PR for these points as this is a bug fix.
I checked and I had already added a test for single series in this PR:
|
Codecov ReportBase: 93.74% // Head: 93.73% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1193 +/- ##
==========================================
- Coverage 93.74% 93.73% -0.01%
==========================================
Files 83 83
Lines 8407 8398 -9
==========================================
- Hits 7881 7872 -9
Misses 526 526
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Summary
add_encoders
in RegressionModels was used in combination with a singletarget
series in fit/predictadd_encoders
) now returns covariates as single TimeSeries when called on single series