-
Notifications
You must be signed in to change notification settings - Fork 80
Conversation
🚀 Deployed on https://deploy-preview-1165--etna-docs.netlify.app |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## inference-v2.1 #1165 +/- ##
=================================================
Coverage ? 87.10%
=================================================
Files ? 164
Lines ? 9070
Branches ? 0
=================================================
Hits ? 7900
Misses ? 1170
Partials ? 0 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
etna/pipeline/base.py
Outdated
@@ -763,6 +780,7 @@ def _run_all_folds( | |||
} | |||
return results | |||
|
|||
# TODO: inconsistency between mode and stride |
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 this. We ignore mode
on n_folds: List[FoldMask]
, but fail on stride
(not ignore).
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.
Can you explain this in more details?
It's expecting behaviour I guess. List[FoldMask]
is proposed to take full control on folds settings
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.
If n_folds: List[FoldMask]
then we ignore mode
parameter because n_folds
takes a full control. But if we set custom stride
, it doesn't ignore it and fails.
But we also have a difference for handling of this parameters:
mode
has some value default value, and we can't fail on it.stride
hasNone
default value that indicates default behavior and setting it manually to someint
can be strange ifn_folds: List[FoldMask]
.
May be we should decide what is better: fail on stride: int
if n_folds: List[FoldMask]
or just ignore 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.
Just ignore I guess
etna/pipeline/base.py
Outdated
@@ -763,6 +780,7 @@ def _run_all_folds( | |||
} | |||
return results | |||
|
|||
# TODO: inconsistency between mode and stride |
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.
Can you explain this in more details?
It's expecting behaviour I guess. List[FoldMask]
is proposed to take full control on folds settings
tests/test_pipeline/test_pipeline.py
Outdated
("constant", 3, 1, None, [0, 7, 14], [-22, -15, -8], [-21, -14, -7], [-15, -8, -1]), | ||
("constant", 3, 2, None, [0, 0, 14], [-22, -22, -8], [-21, -14, -7], [-15, -8, -1]), | ||
("constant", 3, 3, None, [0, 0, 0], [-22, -22, -22], [-21, -14, -7], [-15, -8, -1]), | ||
("constant", 3, 4, None, [0, 0, 0], [-22, -22, -22], [-21, -14, -7], [-15, -8, -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.
Maybe these parameters could be more obvious in case we use horizon
as variable
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, I'll extract it.
Before submitting (must do checklist)
Proposed Changes
Look #1160.
Closing issues
Closes #1160.