Copilot commented on code in PR #45344:
URL: https://github.com/apache/superset/pull/45344#discussion_r4286244912
##########
superset/utils/core.py:
##########
@@ -2188,7 +2188,11 @@ def normalize_dttm_col(
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
)
Review Comment:
The inline comments state that “Data is stored in UTC” and that the code
“First make the datetime UTC-aware”, but the new logic intentionally skips
UTC-localization for tz-aware series (converting from their existing timezone
instead). Please update these comments to match the actual behavior (e.g.,
“assume naive values are UTC; if already tz-aware, convert directly”) to avoid
future misunderstandings/bugs.
##########
tests/unit_tests/utils/test_core.py:
##########
@@ -473,6 +473,25 @@ def test_normalize_dttm_col_timezone_handles_dst() -> None:
assert df["date_col"][0].strftime("%Y-%m-%d %H:%M:%S") == "2020-07-01
02:00:00"
+def test_normalize_dttm_col_timezone_with_tz_aware_values() -> None:
+ """Values that already carry a timezone are converted, not re-localized."""
+ df = pd.DataFrame(
+ {
+ "date_col": pd.to_datetime(
+ ["2020-01-01 00:00:00+00:00", "2020-07-01 05:00:00+05:00"],
+ utc=True,
+ )
+ }
+ )
+ dttm_cols = (DateColumn(col_label="date_col", timezone="Europe/Berlin"),)
+
+ normalize_dttm_col(df, dttm_cols)
+
+ assert is_datetime64_dtype(df["date_col"])
Review Comment:
This assertion verifies a datetime dtype, but it doesn’t explicitly assert
that the timezone info was removed after normalization (a key part of the
behavior via `.dt.tz_localize(None)`). Consider also asserting that the result
is *not* tz-aware (e.g., using `is_datetime64tz_dtype(...)` or checking
`df["date_col"].dt.tz is None`) so the test fails if a future refactor keeps tz
info attached.
--
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]
