SEPURI-SAI-KRISHNA commented on code in PR #45410:
URL: https://github.com/apache/superset/pull/45410#discussion_r4356559903
##########
superset/charts/client_processing.py:
##########
@@ -73,6 +75,32 @@
# The Excel equivalent, applied as a cell format so the value stays numeric.
EXCEL_PERCENT_FORMAT = "0.0%"
+# ``strftime`` copies a directive it does not recognize into its output
+# verbatim, so a d3-only one would reach the report as literal text: ``%L`` and
+# ``%q`` stay ``%L`` and ``%q`` rather than becoming milliseconds and a
quarter.
+# These are the directives that carry the same meaning in d3-time-format and in
+# ``strftime``. Left out are:
+#
+# * the ones that name a weekday, a month or a period (``%a``, ``%A``, ``%b``,
+# ``%B``, ``%p``). The browser reads those names from the d3 locale
+# definition, which ``D3_TIME_FORMAT`` overrides key by key, while
+# ``strftime`` reads them from the worker's ``LC_TIME``. The two agree only
+# when nothing is configured and the worker runs in English.
+# * the locale-composite ones (``%c``, ``%x``, ``%X``), whose output follows
+# the server's locale rather than the chart's.
+# * ``%f``. d3 derives it from ``getUTCMilliseconds() * 1000``, so the
browser's
+# microseconds always end in ``000``, while a warehouse column with
sub-second
+# precision reaches ``strftime`` with the real value.
+# * the platform-dependent or d3-only ones (``%e``, ``%g``, ``%L``, ``%q``,
+# ``%Q``, ``%s``, ``%Z``, ``%z``).
+SHARED_TIME_DIRECTIVES = frozenset("dGHIjmMSuUVwWyY%")
Review Comment:
You are right, and this is the gap I should have caught: I ran the
discrimination check over the branches in `is_strftime_expressible` and treated
`SHARED_TIME_DIRECTIVES` as data rather than as fifteen separate claims.
Dropping `%V` or `%G` does leave the suite green, and the d3 comparison was a
script I ran rather than anything the suite holds.
Added `test_table_time_format_matches_d3_for_each_shared_directive`, one
case per directive so a failure names the one that moved:
```
%d 01 %G 2020 %H 13 %I 01 %j 001 %m 01
%M 05 %S 09 %u 5 %U 00 %V 53 %w 5
%W 00 %y 21 %Y 2021 %% % %G-W%V-%u 2020-W53-5
```
Your suggested value is exactly right: 1 January 2021 is a Friday in ISO
week 53 of 2020, and `%G-W%V-%u` is `2020-W53-5` in both. The timestamp is
2021-01-01 13:05:09 so `%I` separates from `%H` as well. Every expected string
above is what `utcFormat` from `d3-time-format` returns for that instant,
checked against `datetime.strftime` on the same instant.
Removing any single directive from the allowlist now fails its own case.
--
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]
