-
Notifications
You must be signed in to change notification settings - Fork 86
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
Pass X_train and y_train to graph_prediction_vs_actual_over_time #2762
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2762 +/- ##
=======================================
- Coverage 99.8% 99.8% -0.0%
=======================================
Files 301 301
Lines 27897 27889 -8
=======================================
- Hits 27834 27826 -8
Misses 63 63
Continue to review full report at Codecov.
|
6686981
to
672e243
Compare
@@ -7,6 +7,9 @@ Release Notes | |||
* Set ``eval_metric`` to ``logloss`` for ``XGBoostClassifier`` :pr:`2741` | |||
* Added support for ``woodwork`` versions ``0.7.0`` and ``0.7.1`` :pr:`2743` | |||
* Changed ``explain_predictions`` functions to display original feature values :pr:`2759` | |||
* Added ``X_train`` and ``y_train`` to ``graph_prediction_vs_actual_over_time`` and ``get_prediction_vs_actual_over_time_data`` :pr:`2762` | |||
* Added ``forecast_horizon`` as a required parameter to time series pipelines and ``AutoMLSearch`` :pr:`2697` |
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.
Accidentally put the release notes for 2697 into the previous release during a rebase lol
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.
This LGTM! Thank you!
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.
nice!
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 super clean! Just one question to feed my growing time-series knowledge 😂
" xaxis={\"title\": \"Time\"},\n", | ||
" yaxis={\"title\": \"Target Values and Predictions\"},\n", | ||
")\n", | ||
"from evalml.model_understanding import graph_prediction_vs_actual_over_time\n", |
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.
Love how simple this is!
@@ -1427,22 +1427,23 @@ def visualize_decision_tree( | |||
return source_obj | |||
|
|||
|
|||
def get_prediction_vs_actual_over_time_data(pipeline, X, y, dates): | |||
def get_prediction_vs_actual_over_time_data(pipeline, X, y, X_train, y_train, dates): |
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.
Was this a bug? Or simply doing something different? If the latter, is there any functionality that we want to keep around?
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.
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.
Got it, cool stuff !
Pull Request Description
Fixes #2731
After creating the pull request: in order to pass the release_notes_updated check you will need to update the "Future Release" section of
docs/source/release_notes.rst
to include this pull request by adding :pr:123
.