-
Notifications
You must be signed in to change notification settings - Fork 80
Add get_residuals
#597
Add get_residuals
#597
Conversation
Codecov Report
@@ Coverage Diff @@
## master #597 +/- ##
==========================================
+ Coverage 85.13% 85.20% +0.06%
==========================================
Files 117 117
Lines 5806 5818 +12
==========================================
+ Hits 4943 4957 +14
+ Misses 863 861 -2
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
etna/analysis/plotters.py
Outdated
""" | ||
from etna.datasets import TSDataset | ||
|
||
# make flatten dataframes for vectorized operations |
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.
Hm, why we need flatten?
Whats the difference of tsdataset format in case of subtractions?
In both case we subtract one array from another.
And we wouldn't need extra checks.
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.
I think, that checks on the length is still necessary. Ok, I'll try wide-version and check out if validating length or segments is important or not.
new_ts = TSDataset(df=new_df, freq=ts.freq) | ||
new_ts.known_future = ts.known_future | ||
new_ts._regressors = ts.regressors | ||
new_ts.transforms = ts.transforms |
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 seems transforms don't have any sense, do them?
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.
But if we have transforms in the original TSDataset it is strange to remove them.
tests/test_analysis/test_plotters.py
Outdated
def test_get_residuals(ts_fixture, request): | ||
"""Test that get_residuals finds residuals correctly.""" | ||
ts = request.getfixturevalue(ts_fixture) | ||
pipeline = Pipeline( |
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.
I think we don't need the whole pipeline to test this small function.
It's overkill, no?
IMPORTANT: Please do not create a Pull Request without creating an issue first.
Before submitting (must do checklist)
Type of Change
Proposed Changes
Look #589.
Related Issue
#589.
Closing issues
Closes #589.