SEPURI-SAI-KRISHNA commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4385015825
##########
superset/charts/client_processing.py:
##########
@@ -835,6 +863,51 @@ def union_currency_context(
}
+def is_strftime_expressible(d3_time_format: str) -> bool:
+ """Whether ``strftime`` renders a d3 time format the way the browser
does."""
+ if LOCAL_TIME_PREFIX in d3_time_format:
+ # ``TimeFormatterRegistry.get`` strips the marker before building the
+ # formatter, so leaving it in would render it as literal text.
+ return False
+ directives = re.findall(r"%(.)", d3_time_format)
+ if not directives:
+ # Nothing for ``strftime`` to substitute, so it hands the string back
+ # unchanged and every cell would read the format itself. This is also
+ # what rejects the aliases the time format controls offer, which the
+ # browser resolves to a formatter that picks a representation per value
+ # rather than to a format string: ``smart_date`` ("Adaptive
+ # formatting", ``SMART_DATE_ID`` in ``superset-ui-core/time-format``)
+ # among them.
+ return False
+ if "%" in re.sub(r"%.", "", d3_time_format):
+ # A ``%`` no directive consumed, as the trailing one in ``%Y%``. d3
+ # drops it (``2021``) and ``strftime`` copies it (``2021%``), so the
+ # two disagree even where every directive is shared. ``%%`` is not
+ # this case: both read it as one escaped ``%``.
+ return False
+ return 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"``.
+
+ An aware value is read in UTC. ``createD3TimeFormatter`` builds the
+ formatter with ``utcFormat`` unless the format asks for local time, which
+ ``is_strftime_expressible`` rejects, whereas ``strftime`` would use the
+ value's own offset and put a different instant in the report.
+ """
+ values = df[column]
+ if isinstance(values.dtype, pd.DatetimeTZDtype):
+ values = values.dt.tz_convert("UTC")
+ df[column] = values.apply(
+ lambda value: value.strftime(d3_time_format) if pd.notna(value) else
value
Review Comment:
Confirmed, and the trim is the registry's, not d3's. `utcFormat(" %Y ")`
returns `" 2024 "` called directly; `TimeFormatterRegistry.get` trims at
`TimeFormatterRegistry.ts:51` before building the formatter. The gate accepts
`" %Y "`, so the report did render `' 2024 '`.
Stripping at the call site so the gate and the formatting both see the
trimmed
format. The new test parametrizes five whitespace shapes and is the only
failure when the strip is reverted, and when it is narrowed to `lstrip`.
Two limits, in the docstring rather than papered over. `str.strip` is not
`String.prototype.trim`: a leading `U+FEFF` is trimmed in the browser and
kept
here, `U+001C`-`U+001F` and `U+0085` the other way round. And a
whitespace-only
format strips to empty and falls through unformatted, so the report keeps the
timestamp where the chart renders empty. That is the same exit as a format
this
cannot reproduce, but say so if you would rather it matched the registry.
Sibling left out: `NumberFormatterRegistry.get` trims at the same spot while
the number path does not strip, so `format_number_with_config(" .1f ", None,
1234.5678)` returns `"1234.5678"`, not `"1234.6"`. Predates this PR,
different
config key, happy to send separately.
--
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]
