-
Notifications
You must be signed in to change notification settings - Fork 473
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
[Minor] fix future regressor #1585
Changes from 8 commits
b4e5637
2b50d35
1aab28f
973edbc
9f7d1d7
cfde05b
4c2eeee
1c3ed9e
f0af782
bbeee77
55d87f7
66d8085
f6cc116
0af42cd
a3e94db
1887dda
3ce8981
c4cc704
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -749,6 +749,7 @@ | |
def add_country_holidays( | ||
self, | ||
country_name: Union[str, list], | ||
subdivision_name: Optional[Union[str, dict]] = None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like the subdivision commits slipped in here. |
||
lower_window: int = 0, | ||
upper_window: int = 0, | ||
regularization: Optional[float] = None, | ||
|
@@ -766,6 +767,9 @@ | |
---------- | ||
country_name : str, list | ||
name or list of names of the country | ||
subdivision_name : str, dict | ||
name or list of names of the subdivisions (e.g., provinces or states) or | ||
a dictionary where the key is the country name and the value is a list of subdivisions | ||
lower_window : int | ||
the lower window for all the country holidays | ||
upper_window : int | ||
|
@@ -789,6 +793,7 @@ | |
regularization = None | ||
self.config_country_holidays = configure.Holidays( | ||
country=country_name, | ||
subdivision=subdivision_name, | ||
lower_window=lower_window, | ||
upper_window=upper_window, | ||
reg_lambda=regularization, | ||
|
@@ -1102,12 +1107,12 @@ | |
# Only display the plot if the session is interactive, eg. do not show in github actions since it | ||
# causes an error in the Windows and MacOS environment | ||
if matplotlib.is_interactive(): | ||
fig | ||
|
||
self.fitted = True | ||
return metrics_df | ||
|
||
def predict(self, df: pd.DataFrame, decompose: bool = True, raw: bool = False): | ||
def predict(self, df: pd.DataFrame, decompose: bool = True, raw: bool = False, auto_extend=False): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid the linked issues being raised, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. auto_extend for me means that we don't do the cutting at the end. So we cut all the time except for the case where it will cause the whole dataframe to be nan. Can still reverse it |
||
"""Runs the model to make predictions. | ||
|
||
Expects all data needed to be present in dataframe. | ||
|
@@ -1176,7 +1181,7 @@ | |
quantiles=self.config_train.quantiles, | ||
components=components, | ||
) | ||
if periods_added[df_name] > 0: | ||
if not auto_extend and periods_added[df_name] > 0: | ||
fcst = fcst[:-1] | ||
else: | ||
fcst = _reshape_raw_predictions_to_forecst_df( | ||
|
@@ -1191,9 +1196,10 @@ | |
quantiles=self.config_train.quantiles, | ||
config_lagged_regressors=self.config_lagged_regressors, | ||
) | ||
if periods_added[df_name] > 0: | ||
fcst = fcst[: -periods_added[df_name]] | ||
if not auto_extend and periods_added[df_name] > 0: | ||
fcst = fcst[:-1] | ||
forecast = pd.concat((forecast, fcst), ignore_index=True) | ||
|
||
df = df_utils.return_df_in_original_format(forecast, received_ID_col, received_single_time_series) | ||
self.predict_steps = self.n_forecasts | ||
return df | ||
|
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 also seems to come from another PR
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.
Also, this change will always override a set
seasonality_local_reg
and overwrite it to be1
which does not appear intentional?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.
No was trying to solve the flake8 errors - doesn't make any difference anyways