[FEAT] Add support for historical and static exogenous in TimeXer#1404
[FEAT] Add support for historical and static exogenous in TimeXer#1404
Conversation
JQGoh
left a comment
There was a problem hiding this comment.
A couple of questions and requests:
- Suggest to add a minimal example of test using exogenous variables for in
tests/test_models/test_timexer.py. Consider examples like 1) X with static exog, 2) X without static exog - I notice that you include
pydanticdependency in this PR, is it because you ran into Ray-related failure and this helps to resolve it? If that is the case, do you still remember what kind of Ray error?
Otherwise, the PR LGTM
So I added the dependency to fix the issue. |
elephaint
left a comment
There was a problem hiding this comment.
Minor comment, the dependency we need is ray.train, that is missing. So, I'd add the ray.train dependency
|
For the tests, I agree we should have them, but I noticed that none of the models that support exogenous features have tests for exog features. I can make a separate PR to add a global test for models that support exog feat, something similar to the current |
|
yes, I also notice that. Sounds good to me
|
elephaint
left a comment
There was a problem hiding this comment.
LGTM, nice!
non-blocking - you can use "check_model" from .common._model_checks with "losses" to test models against each loss function, this typically uncovers bugs (I didn't run it on this PR, just "fyi" in case you didn't know)
TimeXer now supports all types of exogenous features.
Example usage: