-
Notifications
You must be signed in to change notification settings - Fork 133
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
Bt enhance #83
Bt enhance #83
Conversation
@@ -340,7 +348,7 @@ def _set_score(self, response_col, predicted_col='prediction', metrics=None, | |||
""" | |||
# TODO: not sure should we restrict use of groupby | |||
# groupby is controlled internally with include_steps | |||
groupby = ['steps'] if include_steps else None | |||
groupby = ['df_key', 'steps'] if include_steps else ['df_key'] |
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.
What are the values of column ['df_key'] holding? Could you give an example?
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, now in _score_df
and _predicted_df
we have df_key
(either train
or test
) to indicate the data source. I also gave examples in the backtest notebook.
Looks good to me. Just leave a small comment. |
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.
LGTM
resolve #79;
fix #82
insample_predict = True
and retrieved by.get_insample_predictions
andget_insample_scores
..get_fitted_models
train_end
date to_predicted_df
whendata_col
is available; addn_splits
toscore_df
. As such, unit test for backtest is updated to be passed for such additional columns.