fix(forecast): resolve time grain robustly for Prophet forecasting#42145
fix(forecast): resolve time grain robustly for Prophet forecasting#42145yousoph wants to merge 1 commit into
Conversation
dosubot
Bot
added
change:backend
|
Bito Review Skipped - Source Branch Not Found |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42145 +/- ##
==========================================
- Coverage 65.17% 64.97% -0.20%
==========================================
Files 2753 2784 +31
Lines 154657 157312 +2655
Branches 35484 35841 +357
==========================================
+ Hits 100801 102221 +1420
- Misses 51940 53122 +1182
- Partials 1916 1969 +53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
yousoph
added
the
🎪 ⚡ showtime-trigger-start
There was a problem hiding this comment.
Pull request overview
This PR fixes a forecasting failure mode where the Prophet post-processing step could be invoked without a time_grain, causing an unhandled backend TypeError (500) instead of a readable validation error. It makes time grain resolution more robust on the frontend and makes the backend Prophet post-processing function resilient to missing time_grain in the dispatched options.
Changes:
- Frontend: resolve the effective
time_grainfrom (1) adhoc x-axis columntimeGrain, (2)queryObject.extras.time_grain_sqla, (3)formData.time_grain_sqla, falling back to daily (P1D). - Backend: make
time_grainoptional inprophet()so missing/Nonehits the existing"Time grain missing"InvalidPostProcessingErrorpath rather than raising a rawTypeError. - Tests: add regression coverage for missing/
Nonetime_grainon the backend, and for the new frontend resolution/fallback behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/unit_tests/pandas_postprocessing/test_prophet.py |
Adds regression tests ensuring missing/None time_grain yields a readable InvalidPostProcessingError. |
superset/utils/pandas_postprocessing/prophet.py |
Updates Prophet post-processing signature to accept optional time_grain and preserve graceful error behavior. |
superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts |
Resolves effective time_grain from x-axis/query extras/form data with a P1D fallback to avoid dropped undefined options. |
superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts |
Adds tests for time grain resolution sources and the daily fallback when no grain is provided. |
github-actions
Bot
added
🎪 6c376b7 🚦 building
🎪 ⌛ 48h
|
🎪 Showtime deployed environment on GHA for 6c376b7 • Environment: http://34.214.238.14:8080 (admin/admin) |
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
Thanks @yousoph, LGTM.
Curious about the P1D fallback: for a chart where Time Grain was deliberately cleared to None, assuming daily could put the forecasted points at the wrong cadence versus the actual data. Not blocking, just wondering if that's worth a follow-up.
|
Thanks for the review @rusackas! Fair point on the cadence. Some notes on the blast radius as shipped: the Agreed it's worth a follow-up. The natural improvement is to infer the frequency from the data when the grain is unset — |
Enabling Forecast (Advanced Analytics) on a time-series chart could fail with the opaque error `prophet() missing 1 required positional argument: 'time_grain'` (follow-up to apache#41180). Root cause ---------- The `prophet` post-processing op is dispatched in `QueryObject.exec_post_processing` via `getattr(pandas_postprocessing, operation)(df, **options)`. The frontend `prophetOperator` set `time_grain: formData.time_grain_sqla`; when that is `undefined` (a saved/dashboard chart whose Time Grain was cleared to "None", or a chart using the generic x-axis where the grain lives on the adhoc column's `timeGrain` popover instead of the panel control), `JSON.stringify` drops the key. The options dict then reaches the backend without `time_grain`, so Python raises a raw `TypeError` at call time — before the graceful `if not time_grain: raise InvalidPostProcessingError` guard inside `prophet()` can run (yielding a 500 instead of a readable 4xx). Reproduced ---------- - Backend: `prophet(df, periods=3, confidence_interval=0.9, index="__timestamp")` (no `time_grain`) raised the exact customer `TypeError`. - Frontend: `prophetOperator` with `time_grain_sqla` absent from formData emitted `time_grain: undefined`. Fix --- - Backend (robustness): make `time_grain` an optional kwarg (`Optional[str] = None`, reordered after the required positional args) so any code path missing a grain now surfaces the readable "Time grain missing" `InvalidPostProcessingError` instead of a `TypeError`. - Frontend (make the feature work): resolve the effective grain in order — adhoc x-axis column `timeGrain`, `queryObject.extras.time_grain_sqla` (picks up dashboard-applied grains), then `formData.time_grain_sqla`, with a final `P1D` fallback matching the `time_grain_sqla` control default so a grain-less chart still forecasts. Tests ----- - Backend: added dispatch-path and explicit-`None` cases to `tests/unit_tests/pandas_postprocessing/test_prophet.py`. - Frontend: added fallback, adhoc-column, and extras resolution cases to `prophetOperator.test.ts`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6c376b7 to
0338d53
Compare
github-actions
Bot
added
🎪 0338d53 🚦 building
🎪 0338d53 📅 2026-07-21T23-16
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
🎪 0338d53 🚦 deploying
🎪 0338d53 🚦 running
|
🎪 Showtime deployed environment on GHA for 0338d53 • Environment: http://54.149.38.146:8080 (admin/admin) |
SUMMARY
Enabling Forecast (Advanced Analytics → Predictive Analytics) on a time-series chart could fail with an opaque 500:
Root cause: the
prophetpost-processing op is dispatched viagetattr(pandas_postprocessing, operation)(df, **options)inQueryObject.exec_post_processing. The frontendprophetOperatorsettime_grain: formData.time_grain_sqla; when that value isundefined— e.g. a saved/dashboard chart whose Time Grain control was cleared to "None", or a chart using the generic x-axis where the grain lives on the adhoc column'stimeGrainpopover instead of the panel control —JSON.stringifydrops the key. The backend then callsprophet()withouttime_grainand Python raises a rawTypeErrorat call time, before the gracefulif not time_grain: raise InvalidPostProcessingError(_("Time grain missing"))guard insideprophet()can run.Fix (both ends):
prophetOperator.ts): resolve the effective time grain in priority order — adhoc x-axis columntimeGrain→queryObject.extras.time_grain_sqla(picks up dashboard-applied grains) →formData.time_grain_sqla→P1Dfallback (matching thetime_grain_sqlacontrol default insharedControls), so forecasting works instead of erroring.prophet.py): maketime_grainan optional kwarg (Optional[str] = None), so any remaining path that can't determine a grain surfaces the existing readable "Time grain missing"InvalidPostProcessingError(400) instead of an unhandledTypeError(500). No positional callers exist — the op is always dispatched with keyword options.BEFORE/AFTER
Before: forecast on a chart without
time_grain_sqlain its form data → HTTP 500, rawTypeErrormessage shown in the chart.After: the grain is resolved from the x-axis column / query extras / form data (falling back to
P1D) and the forecast renders; if a grain still can't be determined server-side, the user gets a readable "Time grain missing" error (400).TESTING INSTRUCTIONS
New tests:
time_grainand explicittime_grain=Noneboth raiseInvalidPostProcessingError("Time grain missing") instead ofTypeError.queryObject.extras, and theP1Dfallback when no grain is present anywhere.To reproduce manually: save a Time-series Line Chart, clear Time Grain to "None", enable Forecast under Advanced Analytics, run the chart.
ADDITIONAL INFORMATION
🤖 Generated with Claude Code