Folds number estimation #1279
Folds number estimation #1279
Conversation
|
🚀 Deployed on https://deploy-preview-1279--etna-docs.netlify.app |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #1279 +/- ##
==========================================
- Coverage 88.01% 87.91% -0.10%
==========================================
Files 186 187 +1
Lines 10778 10822 +44
==========================================
+ Hits 9486 9514 +28
- Misses 1292 1308 +16
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
etna/commands/utils.py
Outdated
| Parameters | ||
| ---------- | ||
| pipeline: | ||
| pipeline for which to estimate number of folds. |
There was a problem hiding this comment.
May be it should be written in a capitalized way.
tests/test_commands/test_utils.py
Outdated
| from etna.transforms import MeanTransform | ||
|
|
||
|
|
||
| def run_forecast_test(pipeline, context_size, ts, expected): |
There was a problem hiding this comment.
From the name of the file it isn't clear that this test is made for estimate_max_n_folds. We should rename the file or the test.
etna/commands/utils.py
Outdated
| raise ValueError("Parameter `ts` is required when estimating for backtest method") | ||
|
|
||
| if ts is not None and len(ts.index) == 0: | ||
| raise ValueError("Empty ts is passed!") |
There was a problem hiding this comment.
What is so special about empty ts?
There was a problem hiding this comment.
This can be removed because we checking condition num_points >= horizon + context_size.
|
Don't forget to add changelog before merging the task. |
tests/test_commands/test_utils.py
Outdated
| (10, 5, 5, 5, 1), | ||
| ), | ||
| ) | ||
| def test_estimate_n_folds(num_points, horizon, stride, context_size, expected): |
There was a problem hiding this comment.
May be it should be called test_private_estimate_n_folds
etna/commands/utils.py
Outdated
| pipeline: | ||
| Pipeline for which to estimate number of folds. | ||
| method_name: | ||
| Method name for which to estimate number of folds. |
There was a problem hiding this comment.
I see here indentation of 3 spaces instead of 4.
etna/commands/utils.py
Outdated
| where :math:`num\\_points` is number of points in the dataset, | ||
| :math:`horizon` is length of forecasting horizon, | ||
| :math:`stride` is number of points between folds, | ||
| :math:`context\\_size` is model context size. |
There was a problem hiding this comment.
Context size of pipeline, not model.
Before submitting (must do checklist)
Proposed Changes
Closing issues
closes #1235