Repository navigation
fix(dataset): convert timezone-aware values with the dataset timezone - #45344
Merged
rusackas merged 2 commits intoOct 20, 2026
Merged
Conversation
normalize_dttm_col applies the dataset timezone by localizing the column to UTC and converting it. Columns read from a timezone-aware type (Postgres timestamptz, BigQuery TIMESTAMP, ClickHouse DateTime64 with a zone) already reach the dataframe as tz-aware, and tz_localize raises "Already tz-aware" on them, so any chart that returns such a column errors once the dataset has a timezone set. Only localize naive values and convert the rest as they are.
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Comment on lines
+2191
to
+2195
| ( | ||
| series.dt.tz_localize("UTC") | ||
| if series.dt.tz is None | ||
| else series | ||
| ) |
Contributor
There was a problem hiding this comment.
Suggestion: When mixed UTC offsets produce an object-dtype series, series.dt.tz raises AttributeError, so charts with those values fail instead of converting.
Assessment: 🟠 Major · 🔁 Occurrence: Rarely · 🏷️ Type error
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/utils/core.py
**Line:** 2191:2195
**Comment:**
*Type Error: When mixed UTC offsets produce an object-dtype series, `series.dt.tz` raises `AttributeError`, so charts with those values fail instead of converting.
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
Contributor
CodeAnt PR Risk: Medium Risk
Assessed commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #45344 +/- ##
=======================================
Coverage 83.45% 83.45%
=======================================
Files 3045 3045
Lines 199148 199181 +33
Branches 46438 46445 +7
=======================================
+ Hits 166199 166235 +36
+ Misses 29723 29720 -3
Partials 3226 3226
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:
|
sadpandajoe
requested review from
rusackas and
sadpandajoe
and
a balanced review from Copilot
October 15, 2026 17:15
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open findings
What changed in this PR
Fixes normalize_dttm_col failing on timezone-aware datetime columns when a dataset timezone is configured, by converting tz-aware values instead of re-localizing them.
Changes:
- Update timezone handling in
normalize_dttm_colto onlytz_localize("UTC")for tz-naive series while converting tz-aware series directly. - Add a unit test covering tz-aware datetime inputs to prevent regressions.
| File | Description |
|---|---|
| tests/unit_tests/utils/test_core.py | Adds coverage for tz-aware datetime series conversion behavior. |
| superset/utils/core.py | Prevents Already tz-aware errors by conditional localize vs convert logic. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
2184
to
2198
| tz = ZoneInfo(_col.timezone) | ||
| # Data is stored in UTC, convert to the dataset's configured timezone | ||
| # First make the datetime UTC-aware, then convert to target timezone | ||
| series = df[_col.col_label] | ||
| if not series.empty and series.notna().any(): | ||
| # Convert UTC to target timezone | ||
| df[_col.col_label] = ( | ||
| series.dt.tz_localize("UTC") | ||
| ( | ||
| series.dt.tz_localize("UTC") | ||
| if series.dt.tz is None | ||
| else series | ||
| ) | ||
| .dt.tz_convert(tz) | ||
| .dt.tz_localize(None) # Remove timezone info for display | ||
| ) |
|
|
||
| normalize_dttm_col(df, dttm_cols) | ||
|
|
||
| assert is_datetime64_dtype(df["date_col"]) |
Copilot stopped reviewing on behalf of
sadpandajoe due to an error
October 15, 2026 18:13
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
sadpandajoe
approved these changes
Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
#37014 added a dataset timezone (
"timezone"in the dataset's extra).normalize_dttm_colapplies it by localizing the column to UTC and then converting it to that timezone.That assumes the values are naive. Columns of a timezone-aware type, like Postgres
timestamptz, BigQueryTIMESTAMPor ClickHouseDateTime64with a zone, come out ofSupersetResultSetasdatetime64[ns, tz], andtz_localize("UTC")raisesTypeError: Already tz-aware, use tz_convert to convert.on them. So once a timezone is set on such a dataset, charts that return the column fail.This only localizes naive values to UTC and converts tz-aware values as they are. Naive columns behave the same as before.
This touches the same block as #45203 (DATE columns), but the two changes are independent.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
timestamptzcolumn, set the dataset extra to{"timezone": "America/New_York"}.pytest tests/unit_tests/utils/test_core.py -k tz_awarefails on master and passes with this change.ADDITIONAL INFORMATION