SEPURI-SAI-KRISHNA commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4322301027


##########
superset/charts/client_processing.py:
##########
@@ -835,6 +852,26 @@ def union_currency_context(
 }
 
 
+def is_strftime_expressible(d3_time_format: str) -> bool:
+    """Whether every directive in a d3 time format means the same to 
strftime."""
+    directives = re.findall(r"%(.)", d3_time_format)
+    return bool(directives) and all(
+        directive in SHARED_TIME_DIRECTIVES for directive in directives
+    )
+
+
+def format_time_column(df: pd.DataFrame, column: Any, d3_time_format: str) -> 
None:
+    """Format a temporal column in place with an explicit d3 time format.
+
+    The caller gates this on ``is_strftime_expressible``, so every directive in
+    ``d3_time_format`` means the same thing here as it does in the browser. 
Null
+    values are left as-is rather than rendered as the string ``"NaT"``.
+    """
+    df[column] = df[column].apply(
+        lambda value: value.strftime(d3_time_format) if pd.notna(value) else 
value

Review Comment:
   Right, and with your numbers:
   
   ```
   strftime as is       : 2024-01-01 00:30
   strftime after UTC   : 2023-12-31 19:00
   ```
   
   `createD3TimeFormatter` is where it is decided, and UTC is the default:
   
   ```ts
     if (typeof locale === 'undefined') {
       const format = useLocalTime ? timeFormat : utcFormat;
       formatFunc = format(formatString);
     } else {
       const localeObject = timeFormatLocale(locale);
       formatFunc = useLocalTime
         ? localeObject.format(formatString)
         : localeObject.utcFormat(formatString);
     }
   ```
   
   `useLocalTime` only comes from the `local!` marker, which is rejected in the 
sibling comment, so every format this post-processor accepts is a UTC one. 
Aware values are converted before formatting:
   
   ```python
       values = df[column]
       if isinstance(values.dtype, pd.DatetimeTZDtype):
           values = values.dt.tz_convert("UTC")
   ```
   
   The test on the dtype rather than on each value is deliberate: 
`is_datetime64_any_dtype` gates the caller, so the column is either 
`datetime64[ns]` with no offset or `datetime64[ns, tz]` with one offset for the 
whole column. A mixed-offset column arrives as object dtype and is never 
formatted here at all.
   
   `test_table_time_format_reads_an_aware_value_in_utc` asserts `["2023-12-31 
19:00"]` for `00:30+05:30`, and fails with `2024-01-01 00:30` without the 
conversion.



-- 
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]

Reply via email to