Add Prophet to AutoML #2619
Add Prophet to AutoML #2619
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2619 +/- ##
=======================================
+ Coverage 99.9% 99.9% +0.1%
=======================================
Files 300 300
Lines 27442 27448 +6
=======================================
+ Hits 27398 27404 +6
Misses 44 44
Continue to review full report at Codecov.
|
# Conflicts: # evalml/tests/component_tests/test_arima_regressor.py
…alml into Add-Prophet-To-AutoML
# Conflicts: # evalml/tests/pipeline_tests/test_pipelines.py # evalml/utils/gen_utils.py
# Conflicts: # .github/workflows/linux_unit_tests_with_latest_deps.yml
| if add_datetime_featurizer and estimator_class.model_family not in [ | ||
| ModelFamily.ARIMA, | ||
| ModelFamily.PROPHET, | ||
| ]: |
There was a problem hiding this comment.
DateTimeFeaturizer drops the date_index column
ParthivNaresh
requested review from
angela97lin,
bchen1116,
christopherbunn,
chukarsten,
dsherry,
eccabay,
freddyaboulton and
jeremyliweishih
and removed request for
chukarsten and
dsherry
| virtualenv test_python -q | ||
| source test_python/bin/activate | ||
| pip install cmdstan-builder==0.0.4 | ||
| pip install prophet-builder |
There was a problem hiding this comment.
Might have to revert usage of prophet-builder this let's not celebrate just yet haha
jeremyliweishih
left a comment
jeremyliweishih
left a comment
There was a problem hiding this comment.
this looks good to me! Excited to see more iterations and more perf testing to come!
freddyaboulton
left a comment
freddyaboulton
left a comment
There was a problem hiding this comment.
Thanks for this @ParthivNaresh ! I agree with the conclusions in the performance tests. I left some suggestions I'd like to resolve before merging.
evalml/pipelines/components/estimators/regressors/prophet_regressor.py
Outdated
Show resolved
Hide resolved
| elif isinstance(y.index, pd.DatetimeIndex): | ||
| date_col = y.reset_index() | ||
| date_col = date_col["index"] | ||
| date_column = X.pop(date_column) |
There was a problem hiding this comment.
How come we need to make all these changes to this method? Should we add unit tests to document the cases where this method would fail prior to this PR?
There was a problem hiding this comment.
This was mainly just a reworking to segment out the logic of creating a prophet dataframe; all the current tests cover this behaviour
…ttps://github.com/alteryx/evalml into Add-Prophet-To-AutoML
Fixes #2579
Perf tests here
Another issue has been filed for the docs