You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org> on 2026/08/15 04:38:06 UTC
[PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
SEPURI-SAI-KRISHNA opened a new pull request, #43205:
URL: https://github.com/apache/superset/pull/43205
<!-- PR TITLE: fix(chart): map the remaining time grains for Prophet forecasting -->
### SUMMARY
Enabling **Forecast** (Advanced Analytics → Predictive Analytics) fails with
`Unsupported time grain` for five time grains that Superset itself offers in
the Time Grain control.
`PROPHET_TIME_GRAIN_MAP` covered 16 of the 21 `TimeGrain` values. `prophet()`
rejects anything missing from it:
```python
if time_grain not in PROPHET_TIME_GRAIN_MAP:
raise InvalidPostProcessingError(_("Unsupported time grain: %(time_grain)s", ...))
```
The five gaps, and how reachable each is — every one is exposed by real engine
specs through their `_time_grain_expressions`:
| TimeGrain | ISO | engine specs offering it |
| --- | --- | --- |
| `FIVE_SECONDS` | `PT5S` | 40 (Postgres, CockroachDB, SQLite, DataFusion, …) |
| `THIRTY_SECONDS` | `PT30S` | 42 (Postgres, CockroachDB, SQLite, DataFusion, …) |
| `HALF_HOUR` | `PT0.5H` | 15 (SQLite, Presto, GSheets, Kusto, …) |
| `SIX_HOURS` | `PT6H` | 15 (SQLite, Presto, Druid, GSheets, …) |
| `QUARTER_YEAR` | `P0.25Y` | 10 (SQLite, Shillelagh, GSheets, Ocient, …) |
Three of them are also advertised as valid by the API contract:
`ChartDataProphetOptionsSchema.time_grain` validates with
`validate.OneOf(choices=get_time_grain_choices())`, and `PT5S`, `PT30S` and
`PT6H` are all in `builtin_time_grains`. So the OpenAPI spec publishes them as
accepted values for the forecast operation, and the request then fails.
Two of the five are simply alternate ISO-8601 spellings of grains that already
work — `PT0.5H` is `PT30M`, `P0.25Y` is `P3M` — so picking one spelling
forecasts and the other errors, for the same interval.
Reproducing on `master`:
```python
>>> prophet(df=prophet_df, time_grain="PT6H", periods=3, confidence_interval=0.9)
InvalidPostProcessingError: Unsupported time grain: PT6H
```
This PR maps all five to their pandas frequency aliases (`5s`, `30s`, `30min`,
`6h`, and the same quarter alias `QUARTER` already uses). No `TimeGrain` value
is left unmapped.
Related but distinct: #42145 fixed `prophet()` being called with a *missing*
`time_grain`. This fixes grains that are present and valid but unmapped.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only fix.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/pandas_postprocessing/test_prophet.py
```
Three tests are added:
| test | asserts |
| --- | --- |
| `test_prophet_previously_unmapped_time_grains` | all five grains now produce a forecast (parametrized; fails on `master`) |
| `test_prophet_every_time_grain_is_mapped` | no `TimeGrain` is missing from the map, so this cannot regress |
| `test_prophet_alias_time_grains_match_their_canonical_form` | `HALF_HOUR`/`QUARTER_YEAR` resolve identically to `THIRTY_MINUTES`/`QUARTER` |
Manually: on a Postgres or SQLite dataset, build a time-series chart, set Time
Grain to **6 hour** (or **5 second** / **30 second**), then enable Forecast
under Advanced Analytics. On `master` the chart errors with "Unsupported time
grain"; with this change it forecasts.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### CHECKLIST
- [ ] CI checks pass
- [x] Tests added/updated
- [ ] Documentation updated
- [x] PR title follows conventions
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43205:
URL: https://github.com/apache/superset/pull/43205#issuecomment-5300560526
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #43205:
URL: https://github.com/apache/superset/pull/43205#discussion_r3818875556
##########
superset/utils/pandas_postprocessing/utils.py:
##########
@@ -86,16 +86,24 @@
PROPHET_TIME_GRAIN_MAP: dict[str, str] = {
Review Comment:
Opened as #43356.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #43205:
URL: https://github.com/apache/superset/pull/43205#discussion_r3788672001
##########
superset/utils/pandas_postprocessing/utils.py:
##########
@@ -86,16 +86,24 @@
PROPHET_TIME_GRAIN_MAP: dict[str, str] = {
Review Comment:
Correct, and pre-existing, get_time_grain_choices() merges TIME_GRAIN_ADDONS, so operator-defined grains are advertised but unmapped. That can't be fixed by extending a static map, since addon keys are arbitrary ISO-8601 durations; it needs a general duration→pandas-frequency converter. Out of scope here, where the concern is the built-in grains Superset ships and engine specs already offer. Happy to open a follow-up issue.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43205:
URL: https://github.com/apache/superset/pull/43205#issuecomment-5300579282
<!-- Bito Reply -->
The flagged issue is correct. The `PROPHET_TIME_GRAIN_MAP` in `superset/utils/pandas_postprocessing/utils.py` was missing mappings for several time grains, which caused `prophet()` to fail when encountering them, despite them being valid according to the schema. The provided PR diff already includes a fix by adding these missing mappings to `PROPHET_TIME_GRAIN_MAP` and adding unit tests to ensure all `TimeGrain` values are covered.
Since the fix is already implemented in the PR, no further action is required for this specific comment.
**superset/utils/pandas_postprocessing/utils.py**
```
PROPHET_TIME_GRAIN_MAP: dict[str, str] = {
TimeGrain.SECOND: "s",
+ TimeGrain.FIVE_SECONDS: "5s",
+ TimeGrain.THIRTY_SECONDS: "30s",
TimeGrain.MINUTE: "min",
TimeGrain.FIVE_MINUTES: "5min",
TimeGrain.TEN_MINUTES: "10min",
TimeGrain.FIFTEEN_MINUTES: "15min",
TimeGrain.THIRTY_MINUTES: "30min",
+ # An alternate ISO-8601 spelling of THIRTY_MINUTES that a number of engine
+ # specs expose instead; the two denote the same interval.
+ TimeGrain.HALF_HOUR: "30min",
TimeGrain.HOUR: "h",
+ TimeGrain.SIX_HOURS: "6h",
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43205:
URL: https://github.com/apache/superset/pull/43205#issuecomment-5300576187
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43205?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.26%. Comparing base ([`21ae918`](https://app.codecov.io/gh/apache/superset/commit/21ae918656c808c43df7214a7979f85800f4fc43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b6eee23`](https://app.codecov.io/gh/apache/superset/commit/b6eee23f31565b6bea21e5bdd532fb1c73d35d17?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (21ae918) and HEAD (b6eee23). Click for more details.
>
> <details><summary>HEAD has 55 uploads less than BASE</summary>
>
>| Flag | BASE (21ae918) | HEAD (b6eee23) |
>|------|------|------|
>|python|44|2|
>|presto|8|1|
>|hive|7|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43205 +/- ##
==========================================
- Coverage 66.65% 57.26% -9.39%
==========================================
Files 2873 2873
Lines 163626 163626
Branches 37767 37767
==========================================
- Hits 109059 93695 -15364
- Misses 52438 69042 +16604
+ Partials 2129 889 -1240
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.17% <ø> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.11% <ø> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.17% <ø> (-19.10%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/43205/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43205?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on PR #43205:
URL: https://github.com/apache/superset/pull/43205#issuecomment-5450511249
Gentle nudge on this one — it's been open since Aug 15 without a first review.
It's the smallest change in this area I have open: +54/-0 across two files, 46 of which are tests. No behaviour change beyond the added mappings.
`PROPHET_TIME_GRAIN_MAP` holds 16 keys against 19 built-in time grains, so `PT5S`, `PT30S` and `PT6H` pass schema validation and then fail inside `prophet()`. This brings the map to 21 keys, making it a superset of the built-ins.
That last part also unblocks #43356: deriving the published `time_grain` choices from the map is only safe once the map covers every built-in grain, otherwise those three silently drop out of the spec. CI is green (62 passing).
No urgency — happy to rebase or split if that helps.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43205:
URL: https://github.com/apache/superset/pull/43205#discussion_r3788510024
##########
superset/utils/pandas_postprocessing/utils.py:
##########
@@ -86,16 +86,24 @@
PROPHET_TIME_GRAIN_MAP: dict[str, str] = {
Review Comment:
**Suggestion:** The mapping still contains only built-in `TimeGrain` values, while `ChartDataProphetOptionsSchema` also accepts keys from `TIME_GRAIN_ADDONS`. A configured custom grain will pass schema validation and then fail in `prophet()` at the direct map lookup, so either custom grains need a supported pandas-frequency mapping or Prophet's schema must exclude them. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Forecasting fails for configured custom time grains.
- ⚠️ API validation advertises unsupported forecast values.
- ⚠️ Chart queries return validation errors instead of forecasts.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ee8cebccc684498cb2de9971dea844a5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ee8cebccc684498cb2de9971dea844a5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/pandas_postprocessing/utils.py
**Line:** 87:112
**Comment:**
*Api Mismatch: The mapping still contains only built-in `TimeGrain` values, while `ChartDataProphetOptionsSchema` also accepts keys from `TIME_GRAIN_ADDONS`. A configured custom grain will pass schema validation and then fail in `prophet()` at the direct map lookup, so either custom grains need a supported pandas-frequency mapping or Prophet's schema must exclude them.
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
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43205&comment_hash=494e397bc695a7af3415e44ef43bbfe667665d76fb767dca8e331c67bcf97f44&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43205&comment_hash=494e397bc695a7af3415e44ef43bbfe667665d76fb767dca8e331c67bcf97f44&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(chart): map the remaining time grains for Prophet forecasting [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #43205:
URL: https://github.com/apache/superset/pull/43205
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org