villebro commented on code in PR #19554:
URL: https://github.com/apache/superset/pull/19554#discussion_r882403129
##########
superset/utils/pandas_postprocessing/forecast.py:
##########
@@ -133,14 +151,19 @@ def prophet( # pylint: disable=too-many-arguments
if len(df.columns) < 2:
raise InvalidPostProcessingError(_("DataFrame include at least one
series"))
- target_df = DataFrame()
- for column in [column for column in df.columns if column != index]:
- fit_df = _prophet_fit_and_predict(
- df=df[[index, column]].rename(columns={index: "ds", column: "y"}),
+ target_df = pd.DataFrame()
+ ds = df[DTTM_ALIAS]
+ for column in [column for column in df.columns if column != DTTM_ALIAS]:
Review Comment:
We should reference the index rather than the fixed `DTTM_ALIAS` (this is
relevant when using `GENERIC_CHART_AXES`)
```suggestion
ds = df[index]
for column in [column for column in df.columns if column != index]:
```
##########
superset/utils/pandas_postprocessing/forecast.py:
##########
@@ -133,14 +151,19 @@ def prophet( # pylint: disable=too-many-arguments
if len(df.columns) < 2:
raise InvalidPostProcessingError(_("DataFrame include at least one
series"))
- target_df = DataFrame()
- for column in [column for column in df.columns if column != index]:
- fit_df = _prophet_fit_and_predict(
- df=df[[index, column]].rename(columns={index: "ds", column: "y"}),
+ target_df = pd.DataFrame()
+ ds = df[DTTM_ALIAS]
+ for column in [column for column in df.columns if column != DTTM_ALIAS]:
+ model = forecasts.get_model(
+ model_name=model_name,
confidence_interval=confidence_interval,
- yearly_seasonality=_prophet_parse_seasonality(yearly_seasonality),
- weekly_seasonality=_prophet_parse_seasonality(weekly_seasonality),
- daily_seasonality=_prophet_parse_seasonality(daily_seasonality),
+ yearly_seasonality=_parse_seasonality(yearly_seasonality,
"yearly", ds),
+ monthly_seasonality=_parse_seasonality(monthly_seasonality,
"monthly", ds),
+ weekly_seasonality=_parse_seasonality(weekly_seasonality,
"weekly", ds),
+ daily_seasonality=_parse_seasonality(daily_seasonality, "daily",
ds),
+ )
+ fit_df = model.fit_transform( # type: ignore
+ df=df[[DTTM_ALIAS, column]].rename(columns={DTTM_ALIAS: "ds",
column: "y"}),
Review Comment:
same here
```suggestion
df=df[[index, column]].rename(columns={index: "ds", column:
"y"}),
```
##########
superset/utils/pandas_postprocessing/forecast.py:
##########
@@ -157,4 +180,4 @@ def prophet( # pylint: disable=too-many-arguments
for new_column in new_columns:
target_df = target_df.assign(**{new_column:
fit_df[new_column]})
target_df.reset_index(level=0, inplace=True)
- return target_df.rename(columns={"ds": index})
+ return target_df.rename(columns={"ds": DTTM_ALIAS})
Review Comment:
and here:
```suggestion
return target_df.rename(columns={"ds": index})
```
##########
superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts:
##########
@@ -121,19 +121,20 @@ interface _PostProcessingPivot {
}
export type PostProcessingPivot = _PostProcessingPivot | DefaultPostProcessing;
-interface _PostProcessingProphet {
- operation: 'prophet';
+interface _PostProcessingForecast {
+ operation: 'forecast';
options: {
time_grain: TimeGranularity;
periods: number;
confidence_interval: number;
yearly_seasonality?: boolean | number;
weekly_seasonality?: boolean | number;
daily_seasonality?: boolean | number;
+ model_name: string;
};
}
Review Comment:
As renaming the post processing op is a breaking change, I think we should
migrate existing the `query_context` column in the `slices` table as follows:
- update operation from `prophet` to `forecast` where there exists
`"operation": "prophet"`
- while we're at it, maybe also explicitly set `model_name` to
`prophet.Prophet`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
