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
Add tests for models
on data with integer timestamp
#188
Conversation
🚀 Deployed on https://deploy-preview-188--etna-docs.netlify.app |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unaligned-data #188 +/- ##
=================================================
Coverage ? 89.87%
=================================================
Files ? 198
Lines ? 13172
Branches ? 0
=================================================
Hits ? 11838
Misses ? 1334
Partials ? 0 ☔ View full report in Codecov by Sentry. |
(NaiveModel(lag=3), []), | ||
(NBeatsInterpretableModel(input_size=7, output_size=7, trainer_params=dict(max_epochs=1)), []), | ||
(NBeatsGenericModel(input_size=7, output_size=7, trainer_params=dict(max_epochs=1)), []), | ||
(RNNModel(input_size=2, encoder_length=7, decoder_length=7, trainer_params=dict(max_epochs=1)), []), |
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.
We should discuss is it ok that we should change input_size
from 1
to 2
.
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.
It was decided that it isn't a good behavior and time feature should be ignored even if it is integer.
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.
so we should change it to 1?
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.
Ok, we could change it to 1 and move into test-case with errors. Fixing the error will be done in a separate task.
ts_int_timestamp, model, transforms, method_name="predict", num_skip_points=50 | ||
) | ||
|
||
@to_be_fixed(raises=NotImplementedError, match="Method predict isn't currently implemented") |
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.
Have we not implemented it yet?
what a shame...
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.
Yes, it still isn't implemented. Let's at least create an issue about it.
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.
.
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.
👍
Before submitting (must do checklist)
Proposed Changes
Look #178.
Closing issues
Closes #178.