Rolling Mean features for time series #3028
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3028 +/- ##
=======================================
+ Coverage 99.8% 99.8% +0.1%
=======================================
Files 313 313
Lines 30490 30567 +77
=======================================
+ Hits 30400 30477 +77
Misses 90 90
Continue to review full report at Codecov.
|
freddyaboulton
force-pushed
the
2510-rolling-windows-time-series
branch
3 times, most recently
from
836955e
to
d22d9aa
Compare
freddyaboulton
force-pushed
the
2510-rolling-windows-time-series
branch
from
a98f7d7
to
667d943
Compare
| gap=self.start_delay, | ||
| min_periods=size + 1, | ||
| ) | ||
| rolling_mean = rolling_mean.get_function() |
There was a problem hiding this comment.
I tried computing the features with dfs but I think it's a bit awkward/confusing to have one set of features go through featuretools while the other does not: #3088
I spoke with the featuretools team, and it's probably best to wait until they release Lagged rolling primitives so we can refactor the whole component to use dfs at that point.
freddyaboulton
requested review from
chukarsten,
angela97lin,
bchen1116,
christopherbunn,
dsherry,
eccabay,
jeremyliweishih and
ParthivNaresh
and removed request for
chukarsten
freddyaboulton
changed the title
| @@ -59,6 +66,7 @@ def __init__( | |||
| gap=0, | |||
| forecast_horizon=1, | |||
| conf_level=0.05, | |||
| rolling_window_size=0.25, | |||
There was a problem hiding this comment.
I think this should be added to the docstring!
| def _compute_delays(self, X_ww, y, original_features): | ||
| """Computes the delayed features for all features in X and y. | ||
|
|
||
| Use the autocorrelation to determine delays. |
There was a problem hiding this comment.
I guess if we're going to keep this docstring, we should update.
| max_delay=max_delay, | ||
| forecast_horizon=forecast_horizon, | ||
| gap=gap, | ||
| delay_features=False, |
There was a problem hiding this comment.
Can this just be parameterized into the other test? If you feel it's better tested out explicitly, that's cool too. I assume that's why you did this!
There was a problem hiding this comment.
Yea I liked testing both halves of the feature engineering computation separately to make it easier to verify the output matched the expected value!
| }, | ||
| index=pd.RangeIndex(50, 81), | ||
| ) | ||
| rolling_features_target_only = pd.DataFrame( |
There was a problem hiding this comment.
nit: rolling_features_target_only = rolling_features?
| @@ -59,6 +66,7 @@ def __init__( | |||
| gap=0, | |||
| forecast_horizon=1, | |||
| conf_level=0.05, | |||
| rolling_window_size=0.25, | |||
Pull Request Description
Fixes #2510
Perf tests here
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.rstto include this pull request by adding :pr:123.