Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Shift of exogenous data #1254
Shift of exogenous data #1254
Changes from 15 commits
6a44331
df3f3da
01dc9b0
6b42654
d8fa4c2
7070b18
de77c59
70a7d5f
bdf1b69
3a1a3d9
e62c4c9
6e6572e
826ed0e
bced70b
362ec4b
49ed970
4dd6333
380fb7f
6d4fcb7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why are we passing
df_exog
here? We can only shift something that was indf_exog
?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.
Basically, the use case of this transform is to shift additional regressors when they are not available all the way down to the horizon. Is this transform necessary in case of other types of features?
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 think we will save us from some trouble if we will keep working only with
df_exog
.The problem that we can have
quantiles
indf_exog
in theory. It shouldn't be possible but it can work like this right now...I think we can ingore this problem in this transform for now. It should be fixed on the level of handling quantiles.
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.
You could put logic with checking
None
inside ofself._save_exog_last_date
.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.
Could we really reach this
else
clause?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, for example when dataset has no exog varibales.
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 we rewrite it like this?
Because we have a guarantee that if
self._exog_last_date
isn'tNone
it is dict.It also can't be called with
self._exog_last_date is None
unless it is called directly.