sadpandajoe commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4385565304
##########
superset/charts/client_processing.py:
##########
@@ -1372,6 +1455,31 @@ def label_of(name: str) -> str:
# left untouched, matching the browser.
for column in df.columns:
config = config_by_label.get(column) or {}
+
+ # Mirror the Table plugin's first formatter branch
+ # (``if (isTime || config.d3TimeFormat)`` in ``transformProps.ts``):
+ # an explicit per-column ``d3TimeFormat`` formats a temporal column and
+ # takes precedence over number formatting. Scoped to a datetime column
+ # carrying a format every directive of which ``strftime`` reads the
same
+ # way. A column of another dtype has no ``strftime`` to call, and the
+ # bare ``isTime`` fallback, the ``tableTimestampFormat`` default,
+ # adaptive granularity and string temporal columns are left to a
+ # follow-up: they need a full server-side d3-time-format port rather
+ # than ``strftime``.
+ # ``TimeFormatterRegistry.get`` trims the format before building the
+ # formatter while d3 itself does not, so ``utcFormat(" %Y ")`` returns
+ # ``" 2024 "`` and without the strip the report keeps padding the chart
+ # dropped. One that is only whitespace strips to empty and falls
+ # through unformatted, like one whose directives cannot be reproduced.
+ time_format = (config.get("d3TimeFormat") or "").strip()
Review Comment:
Nothing in the suite pins the `or ""` here. Every table test either omits
`d3TimeFormat` or passes a string, so swapping this for
`config.get("d3TimeFormat", "").strip()` keeps them all green while a column
whose format was cleared and saved as JSON `null` raises `AttributeError:
'NoneType' object has no attribute 'strip'` and fails the whole report. Could
you add a `table()` test with `{"ds": {"d3TimeFormat": None}}` on a datetime
column next to a number-formatted metric, asserting the timestamp is untouched
and the metric still formats?
--
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]