Skip to content

Commit

Permalink
Revert "Add pipeline.should_skip_featurization flag (#3849)" (#3862)
Browse files Browse the repository at this point in the history
* Revert "Add pipeline.should_skip_featurization flag (#3849)"

This reverts commit 61a0a0e.

* release notes
  • Loading branch information
eccabay authored Nov 29, 2022
1 parent 3c91659 commit 40fc693
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 14 deletions.
2 changes: 1 addition & 1 deletion docs/source/release_notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ Release Notes
* Updated demo dataset links to point to new endpoint :pr:`3826`
* Updated ``STLDecomposer`` to infer the time index frequency if it's not present :pr:`3829`
* Updated ``_drop_time_index`` to move the time index from X to both ``X.index`` and ``y.index`` :pr:`3829`
* Added ``TimeSeriesPipeline.should_skip_featurization`` to fix bug where data would get featurized unnecessarily :pr:`3849`
* Fixed bug where engineered features lost their origin attribute in partial dependence, causing it to fail :pr:`3830`
* Fixed bug where partial dependence's fast mode handling for the DFS Transformer wouldn't work with multi output features :pr:`3830`
* Allowed target to be present and ignored in partial dependence's DFS Transformer fast mode handling :pr:`3830`
Expand All @@ -20,6 +19,7 @@ Release Notes
* Removed Featuretools version upper bound and prevent Woodwork 0.20.0 from being installed :pr:`3813`
* Updated min Featuretools version to 0.16.0, min nlp-primitives version to 2.9.0 and min Dask version to 2022.2.0 :pr:`3823`
* Rename issue templates config.yaml to config.yml :pr:`3844`
* Reverted change adding a ``should_skip_featurization`` flag to time series pipelines :pr:`3862`
* Documentation Changes
* Added information about STL Decomposition to the time series docs :pr:`3835`
* Testing Changes
Expand Down
7 changes: 2 additions & 5 deletions evalml/pipelines/time_series_pipeline_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,10 @@ def __init__(
"ARIMA Regressor",
"Prophet Regressor",
]
self.should_skip_featurization = (
self.should_drop_time_index = (
not datetime_featurizer_included
and not time_series_featurizer_included
and self.estimator is not None
)
self.should_drop_time_index = (
self.should_skip_featurization
and self.estimator.name not in time_series_native_estimators
)

Expand Down Expand Up @@ -188,7 +185,7 @@ def transform_all_but_final(self, X, y=None, X_train=None, y_train=None):
X, y = self._convert_to_woodwork(X, y)

empty_training_data = X_train.empty or y_train.empty
if empty_training_data or self.should_skip_featurization:
if empty_training_data or self.should_drop_time_index:
features_holdout = super().transform_all_but_final(X, y)
else:
padded_features, padded_target = self._add_training_data_to_X_Y(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from unittest.mock import patch

import pandas as pd
import pytest
from featuretools import EntitySet, Feature, calculate_feature_matrix, dfs
Expand Down Expand Up @@ -157,9 +155,6 @@ def test_can_run_automl_for_time_series_known_in_advance(
automl.best_pipeline.predict(X_valid, X_train=X, y_train=y)


@patch(
"evalml.pipelines.time_series_pipeline_base.TimeSeriesPipelineBase._add_training_data_to_X_Y",
)
@pytest.mark.parametrize(
"problem_type",
[
Expand All @@ -169,7 +164,6 @@ def test_can_run_automl_for_time_series_known_in_advance(
],
)
def test_can_run_automl_for_time_series_with_exclude_featurizers(
mock_add_X_y,
problem_type,
):

Expand Down Expand Up @@ -218,7 +212,6 @@ def test_can_run_automl_for_time_series_with_exclude_featurizers(
exclude_featurizers=["DatetimeFeaturizer", "TimeSeriesFeaturizer"],
)
automl.search()
assert mock_add_X_y.call_count == 0

rankings = automl.rankings
for score in rankings["validation_score"].values:
Expand All @@ -231,4 +224,3 @@ def test_can_run_automl_for_time_series_with_exclude_featurizers(
assert not pipeline.should_drop_time_index
else:
assert pipeline.should_drop_time_index
assert pipeline.should_skip_featurization

0 comments on commit 40fc693

Please sign in to comment.