Clean up ComponentGraph implementation
#2612
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2612 +/- ##
=======================================
- Coverage 99.9% 99.9% -0.0%
=======================================
Files 297 297
Lines 27039 27033 -6
=======================================
- Hits 26995 26989 -6
Misses 44 44
Continue to review full report at Codecov.
|
angela97lin
requested review from
freddyaboulton,
dsherry,
chukarsten,
bchen1116 and
ParthivNaresh
| @@ -621,7 +621,7 @@ def test_time_series_pipeline_not_fitted_error( | |||
| X, y = X_y_regression | |||
| clf = time_series_regression_pipeline_class( | |||
| parameters={ | |||
| "Linear Regressor": {"n_jobs": 1}, | |||
| "Random Forest Regressor": {"n_jobs": 1}, | |||
There was a problem hiding this comment.
@bchen1116 I can't be 100% sure but due to the occasional issues we're running into with LinearRegressor for time series problems, it might be better to default to a more consistent estimator, but I could be wrong.
There was a problem hiding this comment.
This is trying to use the fixture time_series_regression_pipeline_class which has a Random Forest Regressor, not Linear Regressor 😂 So just cleaning up the parameters so that it's actually used!
@pytest.fixture
def time_series_regression_pipeline_class():
class TSRegressionPipeline(TimeSeriesRegressionPipeline):
"""Random Forest Regression Pipeline for time series regression problems."""
component_graph = ["Delayed Feature Transformer", "Random Forest Regressor"]
def __init__(self, parameters, random_seed=0):
super().__init__(
self.component_graph, parameters=parameters, random_seed=random_seed
)
return TSRegressionPipeline
| parent, component_outputs.get(f"{parent}.x") | ||
| ) | ||
| if isinstance(parent_output, pd.Series): | ||
| parent_output = pd.DataFrame(parent_output, columns=[parent]) |
There was a problem hiding this comment.
Do we no longer need to change the series to a dataframe?
There was a problem hiding this comment.
I think no, this might have been outdated--we use ww's concat_columns and it takes both series and dfs :)
| @@ -621,7 +621,7 @@ def test_time_series_pipeline_not_fitted_error( | |||
| X, y = X_y_regression | |||
| clf = time_series_regression_pipeline_class( | |||
| parameters={ | |||
| "Linear Regressor": {"n_jobs": 1}, | |||
| "Random Forest Regressor": {"n_jobs": 1}, | |||
Closes #2518
I'm still skeptical about
fit_featuresandcompute_final_component_featuresbeing useful but at this point, they're basically just the public facing API for our pipelines to use_fit_transform_features_helper. 🤷♀️