New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: compare time series models #1174
Conversation
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.
Looks good to me. Just change the default metric to smape
and resolve the conflict with my branch and should be good to go.
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 this one - we should avoid using lambda functions unless they are really tiny
@@ -5136,8 +5136,11 @@ def compare_models( | |||
display.display_monitor() | |||
display.display_master_display() | |||
|
|||
# pull() method retrieves metrics id to column the results | |||
id_or_display_name = lambda x: x.id if self._ml_usecase == MLUsecase.TIME_SERIES else x.display_name |
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.
Please turn this into a normal function (with def
)
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.
refactor to id_or_display_name
function
Describe the changes you've made
Major changes include:
compare_models()
support for time series models.create_model_without_cv
switching X, y order before fitting estimator. linkMinor changes include:
0.5.3
, sktime version0.6.0
changes functionality inExpandingWindowSplitter
.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
No tests added
Checklist: