Skip to content

fix(pandas-postprocessing): handle prophet errors and validate minimum data points for forecast #41180

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.

Sign up for GitHub

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

Open
yousoph wants to merge 3 commits into
base: master
Choose a base branch
from
+59 −9
Open

fix(pandas-postprocessing): handle prophet errors and validate minimum data points for forecast #41180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3eacaea
fix(pandas-postprocessing): add error handling and data validation fo…
yousoph Jun 18, 2026
bef607c
fix(pandas-postprocessing): fix ruff pre-commit failures in prophet.py
yousoph Jun 18, 2026
57f98b7
fix(pandas-postprocessing): fix ruff C901 complexity and line-length …
yousoph Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions superset/utils/pandas_postprocessing/prophet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,21 @@ def _prophet_fit_and_predict( # pylint: disable=too-many-arguments
)
if df["ds"].dt.tz:
df["ds"] = df["ds"].dt.tz_convert(None)
model.fit(df)
future = model.make_future_dataframe(periods=periods, freq=freq)
forecast = model.predict(future)[["ds", "yhat", "yhat_lower", "yhat_upper"]]
try:
model.fit(df)
future = model.make_future_dataframe(periods=periods, freq=freq)
forecast = model.predict(future)[["ds", "yhat", "yhat_lower", "yhat_upper"]]
except Exception as ex: # noqa: BLE001
raise InvalidPostProcessingError(
_(
"Unable to generate forecast: %(error)s",
error=str(ex),
)
) from ex
return forecast.join(df.set_index("ds"), on="ds").set_index(["ds"])


def prophet( # pylint: disable=too-many-arguments
def prophet( # pylint: disable=too-many-arguments # noqa: C901
df: DataFrame,
time_grain: str,
periods: int,
Expand Down Expand Up @@ -136,6 +144,8 @@ def prophet( # pylint: disable=too-many-arguments
raise InvalidPostProcessingError(_("DataFrame must include temporal column"))
if len(df.columns) < 2:
raise InvalidPostProcessingError(_("DataFrame include at least one series"))
if len(df) < 2:
raise InvalidPostProcessingError(_("Forecast requires at least 2 data points"))

target_df = DataFrame()

Expand Down
12 changes: 7 additions & 5 deletions superset/utils/pandas_postprocessing/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
from superset.constants import TimeGrain
from superset.exceptions import InvalidPostProcessingError

_PANDAS_VERSION = tuple(int(x) for x in pd.__version__.split(".")[:2])

NUMPY_FUNCTIONS: dict[str, Callable[..., Any]] = {
"average": np.average,
"argmin": np.argmin,
Expand Down Expand Up @@ -76,18 +78,18 @@
)

PROPHET_TIME_GRAIN_MAP: dict[str, str] = {
TimeGrain.SECOND: "S",
TimeGrain.SECOND: "s",
TimeGrain.MINUTE: "min",
TimeGrain.FIVE_MINUTES: "5min",
TimeGrain.TEN_MINUTES: "10min",
TimeGrain.FIFTEEN_MINUTES: "15min",
TimeGrain.THIRTY_MINUTES: "30min",
TimeGrain.HOUR: "H",
TimeGrain.HOUR: "h",
TimeGrain.DAY: "D",
TimeGrain.WEEK: "W",
TimeGrain.MONTH: "M",
TimeGrain.QUARTER: "Q",
TimeGrain.YEAR: "A",
TimeGrain.MONTH: "ME" if _PANDAS_VERSION >= (2, 2) else "M",
TimeGrain.QUARTER: "QE" if _PANDAS_VERSION >= (2, 2) else "Q",
TimeGrain.YEAR: "YE" if _PANDAS_VERSION >= (2, 2) else "A",
TimeGrain.WEEK_STARTING_SUNDAY: "W-SUN",
TimeGrain.WEEK_STARTING_MONDAY: "W-MON",
TimeGrain.WEEK_ENDING_SATURDAY: "W-SAT",
Expand Down
38 changes: 38 additions & 0 deletions tests/unit_tests/pandas_postprocessing/test_prophet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# under the License.
from datetime import datetime
from importlib.util import find_spec
from unittest.mock import patch

import pandas as pd
import pytest
Expand Down Expand Up @@ -186,6 +187,43 @@ def test_prophet_incorrect_time_grain():
)


def test_prophet_insufficient_data():

@codeant-ai-for-open-source codeant-ai-for-open-source Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add an explicit return type annotation to this new test function (for example -> None) to satisfy the rule requiring fully typed new Python functions. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

This is a newly added Python function and it has no return type annotation. The custom rule requires new Python code to be fully typed, so omitting -> None is a real violation.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** tests/unit_tests/pandas_postprocessing/test_prophet.py
**Line:** 190:190
**Comment:**
	*Custom Rule: Add an explicit return type annotation to this new test function (for example `-> None`) to satisfy the rule requiring fully typed new Python functions.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@yousoph yousoph Jun 18, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declining — no existing test functions in this file (or in the broader test suite) use -> None annotations or docstrings. Adding them only to the two new functions would be inconsistent with the established pattern here.

single_row_df = pd.DataFrame(
{
DTTM_ALIAS: [datetime(2022, 1, 1)],
"sales": [100.0],
}
)
with pytest.raises(InvalidPostProcessingError, match="at least 2 data points"):
prophet(
df=single_row_df,
time_grain="P1D",
periods=3,
confidence_interval=0.9,
)


def test_prophet_fit_error():

@codeant-ai-for-open-source codeant-ai-for-open-source Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add a concise docstring under this new function definition to describe the expected error-handling behavior. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

This is a newly added Python function and it has no docstring. That violates the rule requiring new functions and classes to include docstrings.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** tests/unit_tests/pandas_postprocessing/test_prophet.py
**Line:** 206:206
**Comment:**
	*Custom Rule: Add a concise docstring under this new function definition to describe the expected error-handling behavior.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@yousoph yousoph Jun 18, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declining — no existing test functions in this file use docstrings, and adding one only to the new functions would be inconsistent. Test name is self-describing.

@codeant-ai-for-open-source codeant-ai-for-open-source Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Customized review instruction saved!

Instruction:

Do not require docstrings for new test functions when the surrounding test file uses self-describing test names and existing tests in that file omit docstrings; keep the style consistent within the file.

Applied to:

  • tests/unit_tests/pandas_postprocessing/**

💡 To manage or update this instruction, visit: CodeAnt AI Settings

if find_spec("prophet") is None:
pytest.skip("prophet not installed")

with patch(
"superset.utils.pandas_postprocessing.prophet._prophet_fit_and_predict"
) as mock_fit:
mock_fit.side_effect = InvalidPostProcessingError(
"Unable to generate forecast: Dataframe has fewer than 2 non-NaN rows."
)
with pytest.raises(
InvalidPostProcessingError, match="Unable to generate forecast"
):
prophet(
df=prophet_df,
time_grain="P1D",
periods=3,
confidence_interval=0.9,
)


def test_prophet_uncertainty_lower_bound_can_be_negative_for_negative_series():
"""
Regression for #21734: when the input series contains negative values,
Expand Down
Loading